Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

delete pull-related methods from PubSub #1487

Merged
merged 7 commits into from
Dec 22, 2016
Merged

delete pull-related methods from PubSub #1487

merged 7 commits into from
Dec 22, 2016

Conversation

pongad
Copy link
Contributor

@pongad pongad commented Dec 21, 2016

Instead, provide a way to create the Subscriber object.

cc @davidtorres

Instead, provide a way to create the Subscriber object.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 21, 2016
return modifyAckDeadlineAsync(subscription, 0, TimeUnit.SECONDS, ackIds);
}

@Override
public void modifyAckDeadline(String subscription, int deadline, TimeUnit unit, String ackId,

This comment was marked as spam.

.maxQueuedCallbacks(MAX_QUEUED_CALLBACKS.getInteger(optionMap))
.executorFactory(EXECUTOR_FACTORY.getExecutorFactory(optionMap))
public Subscriber subscriber(SubscriptionInfo subscription, Subscriber.MessageReceiver receiver) {
// TODO(pongad): Provide a way to pass in the rest of the options.

This comment was marked as spam.

This comment was marked as spam.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9df40ba on pongad:del-pull into ** on GoogleCloudPlatform:pubsub-hp**.

@pongad
Copy link
Contributor Author

pongad commented Dec 21, 2016

@davidtorres PTAL

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2239a50 on pongad:del-pull into ** on GoogleCloudPlatform:pubsub-hp**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2239a50 on pongad:del-pull into ** on GoogleCloudPlatform:pubsub-hp**.

@garrettjonesgoogle
Copy link
Member

A couple high-level things:

  • We need to write down in our TO DO list to write new samples that replace the deleted ones here (in spirit)
  • Does our new code have the same level of test coverage as the code that's being deleted?
  • We might want to write down some migration details for anyone using the library currently. Could you draft up a list of equivalents to each method on the main surface that you're deleting?

@pongad
Copy link
Contributor Author

pongad commented Dec 22, 2016

@garrettjonesgoogle

  • I made an issue for the TODO list: Tracking issue for PubSub high-perf client integration #1493
  • According to https://coveralls.io/builds/9371615, we have 80%+ on most classes. PollingSubscriberConnection is an exception at 76%. The missed lines are for expoenential backoff. They can be tested relatively easily I think.
  • The migration document is at google-cloud-pubsub/HP-MIGRATION.md, added to the last commit. There isn't that much to document, since the new surface is pretty small.

@garrettjonesgoogle
Copy link
Member

Add a TODO to settle the issue of executors & channels. Otherwise LGTM.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 6cd3804 on pongad:del-pull into ** on GoogleCloudPlatform:pubsub-hp**.

@pongad pongad merged commit 994479f into googleapis:pubsub-hp Dec 22, 2016
@pongad pongad deleted the del-pull branch December 22, 2016 01:17
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 46bf12f on pongad:del-pull into ** on GoogleCloudPlatform:pubsub-hp**.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants