Repository navigation
Pre projects compatibility #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: epic/DM-4993-reland-projects
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -318,21 +318,26 @@ def _delete_if_exists(self, path: str, *, params: Optional[dict] = None) -> None | |
|
|
||
| self._raise_for_status(response) | ||
|
|
||
| def _resolve_project(self, project: Optional[Union[ProjectId, str]]) -> Union[ProjectId, str]: | ||
| def _resolve_project(self, project: Optional[Union[ProjectId, str]]) -> Optional[Union[ProjectId, str]]: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still fails on 3.26.20 to 3.26.22, the current release. This function only needs page_payload = response.json()
active_project_count = page_payload["count"]
if active_project_count != 1:
raise DataMasqueException(
f"No `project` is set and the server has {active_project_count} active projects. Set `project` to choose one."
)
return ProjectId(page_payload["results"][0]["id"])Maybe test with a projects page that has no |
||
| """ | ||
| Returns `project`, else the client's `default_project`, else the ID of the only active project. | ||
|
|
||
| The last is the project the server puts a create into when it is sent no `project`. | ||
| With no active project, or more than one, the server rejects such a create, | ||
| so this raises `DataMasqueException` instead. | ||
| Returns `None` for a server older than projects, which has one set of names and no project filter. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "which has one set of names" is not true for ruleset and discovery config libraries, which are unique per namespace before 3.26.20 (ruleset libraries from 3.26.05, discovery config libraries from 3.26.14). |
||
| """ | ||
|
|
||
| if project is not None: | ||
| return project | ||
| if self.default_project is not None: | ||
| return self.default_project | ||
|
|
||
| response = self.make_request("GET", "/api/projects/", params={"limit": 1}) | ||
| response = self.make_request("GET", "/api/projects/", params={"limit": 1}, require_status_check=False) | ||
| if response.status_code == 404: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: add a test that a status other than 404 (for example 403) still raises |
||
| return None | ||
|
|
||
| self._raise_for_status(response) | ||
| page = Page[Project].model_validate(response.json()) | ||
| if page.count != 1: | ||
| raise DataMasqueException( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,7 @@ def get_discovery_config_library(self, library_id: DiscoveryConfigLibraryId) -> | |
| return DiscoveryConfigLibrary.model_validate(response.json()) | ||
|
|
||
| def _get_discovery_config_library_id_by_name( | ||
| self, name: str, project: Union[ProjectId, str] | ||
| self, name: str, project: Optional[Union[ProjectId, str]] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When So if the server has
items = response.json()
if project is None:
# A server without projects keys libraries on `(namespace, name)`,
# and a create from this client goes into the default namespace.
items = [item for item in items if item.get("namespace", "") == ""]
entries = [DiscoveryConfigLibrary.model_validate(item) for item in items] |
||
| ) -> Optional[DiscoveryConfigLibraryId]: | ||
| """Returns the ID of the library with the given name in the given project, or `None` if there is none.""" | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,12 +142,13 @@ class RunInfo(BaseModel): | |
| start_time: Optional[datetime] = None | ||
| end_time: Optional[datetime] = None | ||
| options: Optional[dict[str, Any]] = None | ||
| project: str | ||
| project: Optional[str] = None | ||
| """ | ||
| Its project's `<id>-<slug>` reference, e.g. `1-default`. | ||
|
|
||
| The project of the run's ruleset, else its discovery config, else its source connection. | ||
| The run moves when that resource moves to another project. | ||
| `None` from a server older than projects. | ||
| """ | ||
|
|
||
| @model_validator(mode="before") | ||
|
|
@@ -166,8 +167,8 @@ class UnfinishedRun(BaseModel): | |
| ruleset_name: str | ||
| status: MaskingRunStatus | ||
| destination_connection: Optional[RunConnectionRef] = None | ||
| project: str | ||
| """Its project's `<id>-<slug>` reference, e.g. `1-default`.""" | ||
| project: Optional[str] = None | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: only |
||
| """Its project's `<id>-<slug>` reference, e.g. `1-default`. `None` from a server older than projects.""" | ||
|
|
||
| @model_validator(mode="before") | ||
| @classmethod | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,9 @@ def get_ruleset_library(self, library_id: RulesetLibraryId) -> RulesetLibrary: | |
| response = self.make_request("GET", f"/api/ruleset-libraries/{library_id}/") | ||
| return RulesetLibrary.model_validate(response.json()) | ||
|
|
||
| def _get_ruleset_library_id_by_name(self, name: str, project: Union[ProjectId, str]) -> Optional[RulesetLibraryId]: | ||
| def _get_ruleset_library_id_by_name( | ||
| self, name: str, project: Optional[Union[ProjectId, str]] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same problem as in Keep params = {"name_exact": name, "project": project, "limit": 1}
if project is None:
# A server without projects keys libraries on `(namespace, name)`.
# Ordering by namespace puts the default namespace first.
params["ordering"] = "namespace"
response = self.make_request("GET", "/api/ruleset-libraries/", params=params)
page_payload = response.json()
if project is None:
page_payload["results"] = [item for item in page_payload["results"] if item.get("namespace", "") == ""]
page = Page[RulesetLibrary].model_validate(page_payload) |
||
| ) -> Optional[RulesetLibraryId]: | ||
| """Return the ID of the library with the given name in the given project, or `None` if there is none.""" | ||
|
|
||
| response = self.make_request( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| from tests.helpers import active_projects_page, database_connection_config | ||
|
|
||
| BASE_URL = "http://test-server" | ||
| EMPTY_PAGE = {"count": 0, "next": None, "previous": None, "results": []} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
|
|
||
| def make_client(default_project: Optional[str] = None) -> DataMasqueClient: | ||
|
|
@@ -41,36 +42,41 @@ def create_connection(client: DataMasqueClient, project: Optional[str], m: reque | |
|
|
||
|
|
||
| def create_discovery_config(client: DataMasqueClient, project: Optional[str], m: requests_mock.Mocker) -> Any: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These five helpers now call Maybe reverting the five helpers to |
||
| m.get(f"{BASE_URL}/api/discovery/configs/", json=EMPTY_PAGE) | ||
| m.post( | ||
| f"{BASE_URL}/api/discovery/configs/", | ||
| json={"id": "dc-1", "name": "dc", "config_type": "database", "project": "2-acme"}, | ||
| ) | ||
| config = DiscoveryConfig(name="dc", yaml="labels: []\n", config_type="database", project=project) | ||
| return client.create_discovery_config(config) | ||
| return client.create_or_update_discovery_config(config) | ||
|
|
||
|
|
||
| def create_rg_config(client: DataMasqueClient, project: Optional[str], m: requests_mock.Mocker) -> Any: | ||
| m.get(f"{BASE_URL}/api/ruleset-generation-configs/", json=EMPTY_PAGE) | ||
| m.post(f"{BASE_URL}/api/ruleset-generation-configs/", json={"id": "rg-1", "name": "rg", "project": "2-acme"}) | ||
| return client.create_rg_config(RGConfig(name="rg", yaml="x: 1\n", project=project)) | ||
| return client.create_or_update_rg_config(RGConfig(name="rg", yaml="x: 1\n", project=project)) | ||
|
|
||
|
|
||
| def create_table_reference(client: DataMasqueClient, project: Optional[str], m: requests_mock.Mocker) -> Any: | ||
| m.get(f"{BASE_URL}/api/table-references/", json=[]) | ||
| m.post( | ||
| f"{BASE_URL}/api/table-references/", | ||
| json={"id": "tr-1", "name": "tr", "connection": "c-1", "source": "people.csv", "project": "2-acme"}, | ||
| ) | ||
| table_reference = TableReference(name="tr", connection=ConnectionId("c-1"), source="people.csv", project=project) | ||
| return client.create_table_reference(table_reference) | ||
| return client.create_or_update_table_reference(table_reference) | ||
|
|
||
|
|
||
| def create_ruleset_library(client: DataMasqueClient, project: Optional[str], m: requests_mock.Mocker) -> Any: | ||
| m.get(f"{BASE_URL}/api/ruleset-libraries/", json=EMPTY_PAGE) | ||
| m.post(f"{BASE_URL}/api/ruleset-libraries/", json={"id": "rl-1", "name": "rl", "project": "2-acme"}) | ||
| return client.create_ruleset_library(RulesetLibrary(name="rl", yaml="version: '1.0'\n", project=project)) | ||
| return client.create_or_update_ruleset_library(RulesetLibrary(name="rl", yaml="version: '1.0'\n", project=project)) | ||
|
|
||
|
|
||
| def create_discovery_config_library(client: DataMasqueClient, project: Optional[str], m: requests_mock.Mocker) -> Any: | ||
| m.get(f"{BASE_URL}/api/discovery/config-libraries/", json=[]) | ||
| m.post(f"{BASE_URL}/api/discovery/config-libraries/", json={"id": "dl-1", "name": "dl", "project": "2-acme"}) | ||
| return client.create_discovery_config_library( | ||
| return client.create_or_update_discovery_config_library( | ||
| DiscoveryConfigLibrary(name="dl", yaml="labels: []\n", project=project) | ||
| ) | ||
|
|
||
|
|
@@ -114,6 +120,17 @@ def test_a_create_with_no_default_project_sends_no_project(create: Create) -> No | |
| assert "project" not in m.last_request.json() | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("create", CREATES) | ||
| def test_a_create_on_a_server_without_projects_sends_no_project(create: Create) -> None: | ||
| """A server older than projects returns 404 for `/api/projects/`, and a create must still work there.""" | ||
| with requests_mock.Mocker() as m: | ||
| m.get(f"{BASE_URL}/api/projects/", status_code=404) | ||
| create(make_client(), None, m) | ||
|
|
||
| assert "project" not in m.last_request.json() | ||
| assert all("project" not in request.qs for request in m.request_history) | ||
|
|
||
|
|
||
| def test_a_connection_update_sends_no_project() -> None: | ||
| """An update leaves the connection in its own project, whatever the default.""" | ||
| client = make_client(default_project="1-default") | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This holds only when neither the resource's
projectnordefault_projectis set.RulesetSerializerrejects unknown fields ("Got unknown fields") on every release from 2.17.0 to 3.26.19, andRuleLibrarySerializerdoes on 3.26.05 to 3.26.19.So with either set,
create_or_update_rulesetandcreate_or_update_ruleset_librarysendprojectand get a 400.Consider saying that here or editing behaviour?