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.

Sign up for GitHub

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

Jump to bottom

Factor out shared 'Operation' class and helpers. #2165

Merged
merged 9 commits into from
Aug 24, 2016
Merged

Factor out shared 'Operation' class and helpers. #2165

merged 9 commits into from
Aug 24, 2016

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Aug 22, 2016 •
edited

Intended to replace the versions currently in .bigtable.instance and bigtable.cluster. Differences in approach from those:

  • No parsing / reconstruction of operation's name: the new class just holds the name as retrieved.
  • Add a from_pb classmethod factory, which attempts to parse out the metadata Any, IFF the type_url on the Any is set, and there is a class registered for it.
  • Drops the complicated __eq__/__ne__ as YAGNI: if needed, reimplement using just the name attr.

Use new gcloud.operations.Operation for long-running ops.

@tseaver tseaver added api: core api: bigtable Issues related to the Bigtable API. labels Aug 22, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 22, 2016
dhermes
dhermes reviewed Aug 23, 2016
View reviewed changes
docs/index.rst
@@ -74,6 +74,7 @@
bigtable-row
bigtable-row-filters
bigtable-row-data
operation-api

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver
Factor out shared 'Operation' class and helpers.
431d61f 
Intended to replace the versions currently in '.bigtable.instance' and
'bigtable.cluster'.  Differences in approach from those:

- No parsing / reconstruction of operation's 'name':  the new class just
  holds the name as retrieved.

- Add a 'from_pb' classmethod factory, which attempts to parse out the
  'metadata' Any, IFF the 'type_url' on the Any is set, and there is
  a class registered for it.

- Drops the complicated '__eq__'/'__ne__' as YAGNI:  if needed, reimplement
  using just the 'name' attr.

Use new 'gcloud.operations.Operation' for long-running ops.
@tseaver
Add metadata type url registrations.
12b42a6 
Lost in merge conflict resolution.
@tseaver
Move 'operation-api' document to 'core' section.
23660b3
@tseaver
Fix copyright years.
2a501cd
@tseaver
Rename 'Operation.finished' -> 'poll'.
dfa9ac1 
Clarify distinction between that method, which makes an API call to test
for completion, and the 'complete' property, which stores the last result
of that call.
@tseaver
Include 'operation' as top-level module for json-docs.
cf3f860
dhermes
dhermes reviewed Aug 23, 2016
View reviewed changes
gcloud/operation.py
def __init__(self, name, client, metadata=None):
self.name = name
self.client = client
self.metadata = metadata or {}

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Aug 23, 2016

LGTM when Travis goes green

@dhermes
Copy link
Contributor

dhermes commented Aug 23, 2016

Consider squashing some commits

@tseaver
Copy link
Contributor Author

tseaver commented Aug 23, 2016

I'd rather not squash and have to run through Travis again today.

@dhermes
Copy link
Contributor

dhermes commented Aug 23, 2016

Totally

@tseaver
Capture call-site metadata.
01ffb45 
Also, test that operation_pb.metadta is correctly reflected.
@dhermes
Copy link
Contributor

dhermes commented Aug 23, 2016

👍 LGTM after latest commit as well.

@tseaver tseaver merged commit e63b1d8 into googleapis:master Aug 24, 2016
@tseaver tseaver deleted the factor-out-operations branch August 24, 2016 14:34
@dhermes dhermes mentioned this pull request Sep 19, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: bigtable Issues related to the Bigtable API. api: core cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tseaver @dhermes @daspecster @googlebot