-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: adds bundles #161
feat: adds bundles #161
Conversation
Adds support for bundles.find, bundles.find_one, bundles.get, and bundle.delete.
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
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.
A couple of questions but generally makes sense
from .resources import Resources, Resource | ||
|
||
|
||
class BundleMetadata(Resource): |
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.
Does this need to be a Class? It's not a Resource properly, is it? There's nothing you can do with it. It just feels like a nested field in Bundle
to me, Bundle.metadata
is a dict.
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.
I'm considering this a continuation of the pattern of using @property
methods for fields to support compatibility between server versions.
for result in results | ||
] | ||
|
||
def find_one(self) -> Bundle | None: |
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.
find_one()
is pretty pointless without any args or query params, no?
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.
We have defined find
and fine_one
in each of the Resources
classes thus far, it seems ok to continue that pattern. I'm still up for adding fetch
and fetch_one
in place of find
and find_one
when there aren't any supported query params for the endpoint.
Adds support for bundles.find, bundles.find_one, bundles.get, and bundle.delete.
Resolves #109