-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Adding lazy loading support for dataset ID. #667
Conversation
@tseaver I encountered your favorite, I first made a decorator def _lazy_property_deco(method):
return _LazyProperty(method.__name__, method)
class _DefaultsContainer(object):
@_lazy_property_deco
def dataset_id():
return _determine_default_dataset_id() but
Essentially the decorator might put something different in the class definition than the code indicates. If you prefer this approach we can add I kind of like / dislike that |
Shadowing a method with an instance attribute is a perfectly normal, acceptable practice: claiming it is wrong is insisting we ride the Python bike with training wheels on. Let's disable the warning. |
Still need to support connection (and eventually do the same in storage). Again verified only the lazy loading tests used implicit behavior via ```diff diff --git a/gcloud/datastore/_implicit_environ.py b/gcloud/datastore/_implicit_environ.py index 6e61636..6d67be6 100644 --- a/gcloud/datastore/_implicit_environ.py +++ b/gcloud/datastore/_implicit_environ.py @@ -177,6 +177,10 @@ class _LazyProperty(object): self._method = method def __get__(self, obj, objtype): + class FooError(Exception): + pass + raise FooError + if obj is None or objtype is not _DefaultsContainer: return self ```
The _DATASET_ENV_VAR_NAME moved from datastore.__init__ to datastore._implicit_environ.
- Ignoring method-hidden (PyLint) so we can decorate methods in a class. - Making _lazy_property_deco(deferred_callable) to decorate and re-use the function name and use it as a callable - Had to hack around Python2.6 differences in `staticmethod`
4ce0753
to
b9d96ed
Compare
@tseaver PTAL. I just rebased this on top of #666. Some things which differ than the original PR:
|
Changes Unknown when pulling b9d96ed on dhermes:lazy-loading-attempt-4 into * on GoogleCloudPlatform:master*. |
LGTM. |
Adding lazy loading support for dataset ID.
* feat: added baseline model version used to generate the summary feat: added the platform of the virtual agent response messages PiperOrigin-RevId: 556882279 Source-Link: googleapis/googleapis@c8eccda Source-Link: googleapis/googleapis-gen@754eb33 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNzU0ZWIzMzQwZDc2ZTZkYTY5ZjA4MTA0MWQ5N2YwMTgwMjJhNTVkYSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
NOTE: Has #666 as diffbase.
Still need to support connection (and eventually do the same
in storage).
Again verified only the lazy loading tests used implicit
behavior via