defaults to None when attribute doesn't exist

[calvernaz]

Minor fix to Patch when the attribute is not present in the receiver object

File ".../localstack/services/edge.py", line 163, in forward_request
    self._require_service(api)
File ".../localstack/services/edge.py", line 220, in _require_service
    raise HTTPErrorResponse("failed to get service for %s: %s" % (api, e), code=500)
localstack.utils.server.http2_server.HTTPErrorResponse: failed to get service for iam: module 'localstack.services.iam.provider' has no attribute '_validate_resource_syntax'

Reproducible with https://github.com/localstack/localstack-terraform-samples/tree/master/apigatewayv2-ws-sub-protocol

[github-actions[bot]]

LocalStack integration with Pro

       3 files         3 suites   1h 8m 16s :stopwatch: 1 113 tests 1 074 :heavy_check_mark: 39 :zzz: 0 :x: 1 424 runs  1 356 :heavy_check_mark: 68 :zzz: 0 :x:

Results for commit fc61ff5c.

[thrau]

just noticing and taking a note that this line here is wrong and needs to be removed otherwise the exceptions are swallowed: https://github.com/localstack/localstack/blob/fc61ff5cc0a2809a411c0e945738cd15a44478e8/localstack/utils/patch.py#L74

[calvernaz]

• accept that Patch only works for properties that are already there (which is the way it works now, and I think is simple and predictable), and find a different solution for why patch is called with a non-existing attribute in the first place.

The fact that calling in a non-existing attribute is certainly an issue (in this context) and deserves a bit of digging. I notice however that the case in question was called twice, and would work the second time, giving the impression the object is not fully built when the patch is called the first time.

• if we must support attributes that don't exist, i suggest replacing None with a sentinel object, and when we unapply the patch and the old value is the sentinel object, instead of setting it to the old value, simply delete the attribute. this would then change the above code to raise the AttributeError again.

any thoughts?

I think you have a good point for the case we cease the use of the patch, I didn't think we were doing that. But again, for this case in particular the patch is applied a second time for some reason and the attribute exists. I don't have a good answer here, I might be missing enough context to give an educated opinion