Skip to content
This repository was archived by the owner on Mar 20, 2018. It is now read-only.

Conversation

@theacodes
Copy link

  • Add new abstract base class google.gax.future.base.Future.
  • Add base class for polling-style futures google.gax.future.base.PollingFuture.
  • Add class for gRPC-based long-running operations google.gax.future.grpc_operation_future.OperationFuture.
  • Remove google.gax._OperationFuture.
  • Remove google.gax._from_any.
  • Remove dependency on dill.

This decouples the concepts of a Future, a Future that polls, and a Future that polls using gRPC for a LRO. It leaves the door open to the possibility of other types of Futures and also implementing an LRO future without gRPC.

This is a breaking change as written. The code generator will need to be updated to use this new future class. However, based on usage it's likely safe to alias the old name google.gax._OperationFuture to the new google.gax.future.grpc_operation_future.OperationFuture. I can do that if we think it's necessary.

Resolves #194.

/cc @dhermes who I bounced this off of.

* Add new abstract base class google.gax.future.base.Future.
* Add base class for polling-style futures google.gax.future.base.PollingFuture.
* Add class for gRPC-based long-running operations google.gax.future.grpc_operation_future.OperationFuture.
* Remove google.gax._OperationFuture.
* Remove google.gax._from_any.
* Remove dependency on dill.

This is a *breaking* change as written. The code generator will need to be updated to use this new future class. However, based on usage it's likely safe to alias the old name google.gax._OperationFuture to the new google.gax.future.grpc_operation_future.OperationFuture. I can do that if we think it's necessary.
@theacodes theacodes requested a review from lukesneeringer June 28, 2017 04:43
@theacodes
Copy link
Author

Alright, @lukesneeringer, this should be ready for review.

Copy link
Contributor

@lukesneeringer lukesneeringer left a comment

Choose a reason for hiding this comment

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

This is a huge improvement. We can merge it once we update auto-gen to use it (but not before).

@lukesneeringer
Copy link
Contributor

This is a breaking change as written. The code generator will need to be updated to use this new future class. However, based on usage it's likely safe to alias the old name google.gax._OperationFuture to the new google.gax.future.grpc_operation_future.OperationFuture. I can do that if we think it's necessary.

Summoning @dhermes.

Ideally I would prefer not do this and get everything migrated, but that is likely to be something of a process. Would it be better to alias for a couple releases?

@dhermes
Copy link
Contributor

dhermes commented Jul 5, 2017

Why do you think it'd be "something of a process"? We have version pinning for the GAPICs we depend on. Do they in turn have version pinning for GAX?

@theacodes theacodes closed this Jul 17, 2017
@theacodes
Copy link
Author

closing in favor of googleapis/google-cloud-python#3616

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants