Unify parent accessor methods #90
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
09ad12c to
5caec44
Compare
samamorgan
commented
Oct 27, 2023
| except TypeError: | ||
| continue | ||
| @pytest.mark.parametrize("class_name", __all__) | ||
| def test_collection_pageable(client, class_name: str): |
There was a problem hiding this comment.
Note: The change to this test was just to make it parametrized. No functional changes.
samamorgan
commented
Oct 27, 2023
| @@ -45,6 +45,9 @@ def __getattr__(self, name): | |||
| def __setattr__(self, name, value): | |||
| super().__setitem__(name, value) | |||
|
|
|||
| def __delattr__(self, name): | |||
There was a problem hiding this comment.
Added this as a convenience. Previously, if one tried to del an attribute of this class, an AttributeError would be raised. The only way to actually delete an attribute was with dictionary syntax. Now both are possible.
r = Resource(id="123")
del r["id"]
del r.id # Previously not possible
samamorgan
commented
Oct 27, 2023
|
|
||
|
|
||
| class CDT(Resource): | ||
| def create(self): | ||
| @model_id("Patient") | ||
| def create(self, patient_id: str): |
There was a problem hiding this comment.
Adding a positional argument to a method would usually trigger a higher-level version bump. However, this change is technically idempotent and fully backwards-compatible since model_id will set the argument.
samamorgan
commented
Oct 27, 2023
| return f(self, *args, **kwargs) | ||
| except TypeError as exc: | ||
| # Raise from the outer `AttributeError` so we don't lose context. | ||
| raise exc from outer_exc |
There was a problem hiding this comment.
The exception pattern here probably looks a little strange, so let me explain.
At the end of the chain, we do want to raise the normal TypeError that Python would raise if the model ID argument isn't filled. However, we don't want to lose the context so implementers have some hints on how to fix the issue, since this mechanism is doing a bit of recursive introspection. The end result is a traceback chain that includes both the TypeError and the AttributeError(s) like so:
>>> from welkin.models import Patient
>>> Patient().Encounter().EncounterDisposition().get()
Traceback (most recent call last):
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 164, in wrapper
raise e from outer_exc
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 161, in wrapper
kwargs[key] = find_model_id(self, model)
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 135, in find_model_id
return find_model_id(instance._parent, model_name)
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 135, in find_model_id
return find_model_id(instance._parent, model_name)
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 131, in find_model_id
return instance.id
File "/Users/sammorgan/Git/zest-welkin-health/welkin/models/base.py", line 34, in __getattr__
value = super().__getattr__(name)
File "/Users/sammorgan/Git/zest-welkin-health/welkin/models/base.py", line 24, in __getattr__
raise AttributeError(e) from None
AttributeError: 'Patient' object has no attribute 'id'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 164, in wrapper
raise e from outer_exc
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 161, in wrapper
kwargs[key] = find_model_id(self, model)
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 135, in find_model_id
return find_model_id(instance._parent, model_name)
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 131, in find_model_id
return instance.id
File "/Users/sammorgan/Git/zest-welkin-health/welkin/models/base.py", line 34, in __getattr__
value = super().__getattr__(name)
File "/Users/sammorgan/Git/zest-welkin-health/welkin/models/base.py", line 24, in __getattr__
raise AttributeError(e) from None
AttributeError: 'Encounter' object has no attribute 'id'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 172, in wrapper
raise exc from outer_exc
File "/Users/sammorgan/Git/zest-welkin-health/welkin/util.py", line 169, in wrapper
return f(self, *args, **kwargs)
TypeError: EncounterDisposition.get() missing 2 required positional arguments: 'patient_id' and 'encounter_id'aa1f945 to
3a32fd4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We had a number of methods that were introspecting the parent chain for IDs, this cleans all of those up and centralizes all functionality.
This should be backwards-compatible, as no changes were made to existing model tests.
Changelog
model_iddecorator functionPatientsubresources always acceptpatient_idas the first argumentwelkin.utilandwelkin.models.util