Skip to content
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

refact: make collections in apiserver models lazy #343

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Conversation

masci
Copy link
Member

@masci masci commented Oct 29, 2024

Adjust client.apiserver.deployments and client.apiserver.sessions so that they don't actually load the items in a collection, deferring that to list. This way sessions and deployments can be regular properties, accessible without await.
This was originally implemented for the core client, this PR makes it consistent with the rest of the models.

Part of #337

@coveralls
Copy link

coveralls commented Oct 29, 2024

Coverage Status

coverage: 68.017% (+0.07%) from 67.948%
when pulling eee25f5 on massi/apiserver-fix
into 4d4efd7 on main.

return SessionCollection.instance(
make_sync=self._instance_is_sync,
client=self.client,
deployment_id=self.id,
items=items,
items={},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does a SessionCollection (or other collections) even need an items kwarg/attribute on the instance now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, collections don't need it anymore (unless we want to introduce some sort of caching but at this stage I'd rather not)

id=session_def.session_id,
)
for session_def in r.json()
}
return SessionCollection.instance(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me

Conceptually, this is returning a client, and from there, you interact with it

@masci masci merged commit 1ee388b into main Oct 31, 2024
10 checks passed
@masci masci deleted the massi/apiserver-fix branch October 31, 2024 18:19
@masci masci added the enhancement New feature or request label Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants