-
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
Add support for RuntimeConfig API. #2485
Conversation
- `API Documentation`_ | ||
|
||
.. _Homepage: https://googlecloudplatform.github.io/google-cloud-python/ | ||
.. _API Documentation: http://googlecloudplatform.github.io/google-cloud-python/ |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
The Google Cloud `RuntimeConfig`_ (`RuntimeConfig API docs`_) API enables | ||
developers to dynamically configure and expose variables through Google Cloud | ||
Platform. In addition, you can also set Watchers and Waiters (not yet supported | ||
by this client library) that will watch for changes to your data and return |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -0,0 +1,20 @@ | |||
# Copyright 2014 Google Inc. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
# limitations under the License. | ||
|
||
"""Google Cloud Runtime Configurator API package.""" | ||
from google.cloud.runtimeconfig.client import Client |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
"""Extract the config name from a full resource name. | ||
|
||
>>> _config_name_from_full_name('projects/my-proj/configs/my-config') | ||
"my-config" |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
config_name = self._callFUT(PATH) | ||
self.assertEqual(config_name, CONFIG_NAME) | ||
|
||
class Test_variable_name_from_full_name(unittest.TestCase): |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
CONFIG_NAME = 'CONFIG_NAME' | ||
PROJECT = 'my-project-1234' | ||
PATH = 'projects/%s/configs/%s/variables/%s' % ( | ||
PROJECT, CONFIG_NAME, VARIABLE_NAME) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
CONFIG_NAME = 'CONFIG_NAME' | ||
PROJECT = 'my-project-1234' | ||
PATH = 'projects/%s/configs/%s/variables/%s' % ( | ||
PROJECT, CONFIG_NAME, VARIABLE_NAME) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
||
import httplib2 | ||
import six | ||
from six.moves.urllib.parse import quote |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
def test_ctor(self): | ||
client = _Client(project=self.PROJECT) | ||
config = self._makeOne(name=self.CONFIG_NAME, | ||
client=client) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Oops, looks like I mixed tabs and spaces and didn't reach 100% coverage on variable.py. Addressing now. |
Updated, please take a second look. |
I'm getting a lint error
The line is
I don't think there's anything I can do to shrink it down. |
Okay, I think I finally made the linter happy. I shrunk the problematic line down by changing the "names" project-name -> proj-name, config-name -> cfg-name |
|
||
# Variables are hierarchical. Each node in the hierarchy is separated by a | ||
# / character. | ||
return '/'.join(parts[5:]) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
:rtype: string | ||
:returns: The full name based on project and config names. | ||
|
||
:raises: :class:`ValueError` if the config is missing a name. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
``None`` if the property is not set locally. | ||
""" | ||
value = self._properties.get('updateTime') | ||
if value: |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@tswast RE: a line with a string that's too long (like You can use an implicit strong concat in Python: my_var = ('the beginning of the string '
'and here is some more of it') RE: Mixing tabs and spaces, please get rid of all tabs. You can check for them via
then you should find the offending lines via |
except NotFound: | ||
return False | ||
|
||
def reload(self, client=None, fields=None): |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
def test_ctor_w_no_name(self): | ||
client = _Client(project=self.PROJECT) | ||
config = self._makeOne(name=None, client=client) | ||
self.assertRaises(ValueError, lambda: config.full_name) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
client = _Client(project=self.PROJECT) | ||
config = Config(name=self.CONFIG_NAME, client=client) | ||
variable = self._makeOne(name=None, config=config) | ||
self.assertRaises(ValueError, lambda: variable.full_name) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
self.assertEqual(len(conn._requested), 1) | ||
req = conn._requested[0] | ||
self.assertEqual(req['method'], 'GET') | ||
self.assertEqual(req['path'], '/%s' % self.CONFIG_PATH) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Ping. Please take a second look. I believe I have addressed all open comments. |
@dhermes @jonparrott Please review. Is this ready to merge? I just rebased against the latest master. |
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.
@tswast there doesn't seem to be any way to create configs or variables, is this intentional because this is just the first step?
See the ``google-cloud-python`` API `RuntimeConfig documentation`_ to learn | ||
how to analyze images using this library. | ||
|
||
.. _Vision documentation: https://google-cloud-python.readthedocs.io/en/stable/runtimeconfig-usage.html |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This will not make an HTTP request; it simply instantiates | ||
a config object owned by this client. | ||
|
||
:type config_name: string |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
self._properties = {} | ||
|
||
def __repr__(self): | ||
return '<Config: %s>' % (self.name,) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
:rtype: string | ||
:returns: The URL path based on project and config names. | ||
""" | ||
return '/%s' % (self.full_name,) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This will not make an HTTP request; it simply instantiates | ||
a variable object owned by this config. | ||
|
||
:type variable_name: string |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
""" | ||
if not self.name: | ||
raise ValueError('Missing variable name.') | ||
return '%s/variables/%s' % (self.config.full_name, self.name) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
:rtype: string | ||
:returns: The URL path based on config and variable names. | ||
""" | ||
return '/%s' % (self.full_name,) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
def path(self): | ||
"""URL path for the variable's APIs. | ||
|
||
:rtype: string |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
https://cloud.google.com/deployment-manager/runtime-configurator/reference/rest/v1beta1/projects.configs.variables#VariableState | ||
|
||
:rtype: string or ``NoneType`` | ||
:returns: If set, one of "UPDATED", "DELETED", or |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
"my-config" | ||
|
||
:type full_name: string | ||
:param full_name: the full resource name of a config. The full resource |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis#2485 (comment) `str` is a type, `string` is a module.
fa518e1
to
da5092b
Compare
To keep the initial review small, this includes only a minimal set of features. It supports listing and getting variables, but not the other methods of the API.
@jonparrott @dhermes Please take another look. I believe I have addressed all your comments.
Yes. I wanted to keep the initial PR as small as possible to start. Plus, dpebot and the other samples I want to create will just need read access, not write access to RuntimeConfig. After this goes in, we can add creates in, if only so that we can create system tests for this. Right now I'm running the following snippets manually to verify that it does what I want:
|
Broken by simultaneous merges of googleapis#2594 and googleapis#2485.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis#2485 (comment) `str` is a type, `string` is a module.
Add support for RuntimeConfig API.
Broken by simultaneous merges of googleapis#2594 and googleapis#2485.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis/google-cloud-python#2485 (comment) `str` is a type, `string` is a module.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis/google-cloud-python#2485 (comment) `str` is a type, `string` is a module.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis/google-cloud-python#2485 (comment) `str` is a type, `string` is a module.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis/google-cloud-python#2485 (comment) `str` is a type, `string` is a module.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis/google-cloud-python#2485 (comment) `str` is a type, `string` is a module.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis/google-cloud-python#2485 (comment) `str` is a type, `string` is a module.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis/google-cloud-python#2485 (comment) `str` is a type, `string` is a module.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis/google-cloud-python#2485 (comment) `str` is a type, `string` is a module.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: googleapis/google-cloud-python#2485 (comment) `str` is a type, `string` is a module.
Used the command: ag -l 'rtype: string' | xargs sed -i .bak 's/rtype: string/rtype: str/g' Based on this comment: #2485 (comment) `str` is a type, `string` is a module.
Supports listing and getting variables, but not the other methods of the RuntimeConfig API, since
I didn't need those, yet.
First step of #2476