Skip to content

Commit

Permalink
Fix update generation with additionalIdentifiers (#732)
Browse files Browse the repository at this point in the history
* Disable unhelpful pylint warning

pylint insists that `with` should be used on all disposable resources
but in this case the constructor call is in a helper method and the
consuming method does already use `with`. Given that this is a private
method it should be fine to ignore the warning in all cases.

* Fix update generation with additionalIdentifiers

If the `additionalIdentifiers` property is present in a resource schema
the `generate_update_example` method in `resource_client` will not work,
because it checks for unique keys from the create_model and passes
`self._additional_identifiers_paths` to a method that expects a list of
tuples of string, whereas it receives a list of sets of tuples of
string and will fail with

```
AttributeError: 'tuple' object has no attribute 'split'
```

Instead, check if the key is in any of the additional identifiers.

Related issue
-------------

#731
  • Loading branch information
ptrucks authored Apr 28, 2021
1 parent a9ab27a commit 5435570
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 5 deletions.
9 changes: 8 additions & 1 deletion src/rpdk/core/contract/resource_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,14 @@ def get_unique_keys_for_model(self, create_model):
k: v
for k, v in create_model.items()
if self.is_property_in_path(k, self.primary_identifier_paths)
or self.is_property_in_path(k, self._additional_identifiers_paths)
or any(
map(
lambda additional_identifier_paths, key=k: self.is_property_in_path(
key, additional_identifier_paths
),
self._additional_identifiers_paths,
)
)
}

@staticmethod
Expand Down
2 changes: 2 additions & 0 deletions src/rpdk/core/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,8 @@ def _create_context_manager(self, dry_run):
# if it's a dry run, keep the file; otherwise can delete after upload
if dry_run:
return self._get_zip_file_path().open("wb")

# pylint: disable=consider-using-with
return TemporaryFile("w+b")

def _get_zip_file_path(self):
Expand Down
21 changes: 17 additions & 4 deletions tests/contract/test_resource_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@
"primaryIdentifier": ["/properties/c", "/properties/d"],
}

SCHEMA_WITH_ADDITIONAL_IDENTIFIERS = {
"properties": {
"a": {"type": "number"},
"b": {"type": "number"},
"c": {"type": "number"},
"d": {"type": "number"},
},
"readOnlyProperties": ["/properties/b"],
"createOnlyProperties": ["/properties/c"],
"primaryIdentifier": ["/properties/c"],
"additionalIdentifiers": [["/properties/b"]],
}


@pytest.fixture
def resource_client():
Expand Down Expand Up @@ -157,8 +170,8 @@ def resource_client_inputs():
return client


@pytest.fixture
def resource_client_inputs_schema():
@pytest.fixture(params=[SCHEMA_, SCHEMA_WITH_ADDITIONAL_IDENTIFIERS])
def resource_client_inputs_schema(request):
endpoint = "https://"
patch_sesh = patch(
"rpdk.core.contract.resource_client.create_sdk_session", autospec=True
Expand All @@ -181,7 +194,7 @@ def resource_client_inputs_schema():
DEFAULT_FUNCTION,
endpoint,
DEFAULT_REGION,
SCHEMA_,
request.param,
EMPTY_OVERRIDE,
{
"CREATE": {"a": 111, "c": 2, "d": 3},
Expand All @@ -195,7 +208,7 @@ def resource_client_inputs_schema():
mock_account.assert_called_once_with(mock_sesh, {})

assert client._function_name == DEFAULT_FUNCTION
assert client._schema == SCHEMA_
assert client._schema == request.param
assert client._overrides == EMPTY_OVERRIDE
assert client.account == ACCOUNT

Expand Down

0 comments on commit 5435570

Please sign in to comment.