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
Implemented LazyObject class, which provides a 'lazy' class method to create instances with lazy initialization. Th… #665
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
import time | ||
import sys | ||
|
||
from libcloud.common.base import LazyObject | ||
from libcloud.common.google import GoogleResponse | ||
from libcloud.common.google import GoogleBaseConnection | ||
from libcloud.common.google import GoogleBaseError | ||
|
@@ -231,15 +232,37 @@ def page(self, max_results=500): | |
return self | ||
|
||
|
||
class GCELicense(UuidMixin): | ||
class GCELicense(UuidMixin, LazyObject): | ||
"""A GCE License used to track software usage in GCE nodes.""" | ||
def __init__(self, id, name, driver, charges_use_fee, extra=None): | ||
self.id = str(id) | ||
def __init__(self, name, project, driver): | ||
UuidMixin.__init__(self) | ||
self.id = name | ||
self.name = name | ||
self.project = project | ||
self.driver = driver | ||
self.charges_use_fee = charges_use_fee | ||
self.extra = extra or {} | ||
UuidMixin.__init__(self) | ||
self.charges_use_fee = None # init in _request | ||
self.extra = None # init in _request | ||
|
||
self._request() | ||
|
||
def _request(self): | ||
# TODO(crunkleton@google.com): create new connection? or make | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @crunk1 so what is the intention with the todo? to raise a separate PR later There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I need to look into this later. |
||
# connection thread-safe? Saving, modifying, and restoring | ||
# driver.connection.request_path is really hacky and thread-unsafe. | ||
saved_request_path = self.driver.connection.request_path | ||
new_request_path = saved_request_path.replace(self.driver.project, | ||
self.project) | ||
self.driver.connection.request_path = new_request_path | ||
|
||
request = '/global/licenses/%s' % self.name | ||
response = self.driver.connection.request(request, method='GET').object | ||
self.driver.connection.request_path = saved_request_path | ||
|
||
self.extra = { | ||
'selfLink': response.get('selfLink'), | ||
'kind': response.get('kind') | ||
} | ||
self.charges_use_fee = response['chargesUseFee'] | ||
|
||
def destroy(self): | ||
raise ProviderError("Can not destroy a License resource.") | ||
|
@@ -3913,15 +3936,7 @@ def ex_get_license(self, project, name): | |
:return: A DiskType object for the name | ||
:rtype: :class:`GCEDiskType` | ||
""" | ||
saved_request_path = self.connection.request_path | ||
new_request_path = saved_request_path.replace(self.project, project) | ||
self.connection.request_path = new_request_path | ||
|
||
request = '/global/licenses/%s' % (name) | ||
response = self.connection.request(request, method='GET').object | ||
self.connection.request_path = saved_request_path | ||
|
||
return self._to_license(response) | ||
return GCELicense.lazy(name, project, self) | ||
|
||
def ex_get_disktype(self, name, zone=None): | ||
""" | ||
|
@@ -5691,24 +5706,6 @@ def _to_zone(self, zone): | |
maintenance_windows=zone.get('maintenanceWindows'), | ||
deprecated=deprecated, driver=self, extra=extra) | ||
|
||
def _to_license(self, license): | ||
""" | ||
Return a License object from the JSON-response dictionary. | ||
|
||
:param license: The dictionary describing the license. | ||
:type license: ``dict`` | ||
|
||
:return: License object | ||
:rtype: :class:`GCELicense` | ||
""" | ||
extra = {} | ||
extra['selfLink'] = license.get('selfLink') | ||
extra['kind'] = license.get('kind') | ||
|
||
return GCELicense(id=license['name'], name=license['name'], | ||
charges_use_fee=license['chargesUseFee'], | ||
driver=self, extra=extra) | ||
|
||
def _set_project_metadata(self, metadata=None, force=False, | ||
current_keys=""): | ||
""" | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import unittest | ||
import sys | ||
|
||
import mock | ||
|
||
from libcloud.common.base import LazyObject | ||
from libcloud.test import LibcloudTestCase | ||
|
||
|
||
class LazyObjectTest(LibcloudTestCase): | ||
|
||
class A(LazyObject): | ||
def __init__(self, x, y=None): | ||
self.x = x | ||
self.y = y | ||
|
||
def test_lazy_init(self): | ||
# Test normal init | ||
a = self.A(1, y=2) | ||
self.assertTrue(isinstance(a, self.A)) | ||
|
||
# Test lazy init | ||
with mock.patch.object(self.A, | ||
'__init__', return_value=None) as mock_init: | ||
a = self.A.lazy(3, y=4) | ||
self.assertTrue(isinstance(a, self.A)) # Proxy is a subclass of A | ||
mock_init.assert_not_called() | ||
|
||
# Since we have a mock init, an A object doesn't actually get | ||
# created. But, we can still call __dict__ on the proxy, which will | ||
# init the lazy object. | ||
self.assertEqual(a.__dict__, {}) | ||
mock_init.assert_called_once_with(3, y=4) | ||
|
||
def test_setattr(self): | ||
a = self.A.lazy('foo', y='bar') | ||
a.z = 'baz' | ||
wrapped_lazy_obj = object.__getattribute__(a, '_lazy_obj') | ||
self.assertEqual(a.z, 'baz') | ||
self.assertEqual(wrapped_lazy_obj.z, 'baz') | ||
|
||
|
||
if __name__ == '__main__': | ||
sys.exit(unittest.main()) |
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.
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.
Wrapping it in a method was nice, but I don't think you need to use
__getattribute__
to call that method since it's defined in the same class here, so any invocation of this method is guaranteed to haveself._get_lazy_obj()
available to call.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.
Since the Proxy class has
__getattribute__
defined,self._get_lazy_obj()
will first callProxy.__getattribute__(self, '_get_lazy_obj')
to get the bound method. That leads to an infinite recursion loop. See an example here: https://repl.it/Bavg/1.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.
Oh, right, derp. I should stop doing code reviews before I get any caffeine in me.
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.
All good, thanks for all your help!