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.

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 base service option classes for gRPC and HTTP services #1011

Merged
merged 2 commits into from
May 16, 2016

Conversation

mziccard
Copy link
Contributor

This PR follows from the discussion in #1005 and adds base service option classes specific for gRPC and HTTP transports (HttpServiceOptions and GrpcServiceOptions).

This PR also uses the features in the new GrpcServiceOptions base class to initialize DefaultPubSubRpc.

In the future we could consider exposing in GrpcServiceOptions a way of setting the executor, the only issue is see with this is that the class GrpcServiceOptions must be serializable. We could consider not serializing the executor and using the default one (given that we document this behavior and suggest the user to use toBuilder() if he wants to set the executor before starting the service after de-serialization.

@aozarov @ajkannan any kind of feedback is precious.

@mziccard mziccard added api: core api: pubsub Issues related to the Pub/Sub API. labels May 15, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 15, 2016
@mziccard mziccard force-pushed the pubsub-service-options branch 3 times, most recently from 1a35b69 to 6837e91 Compare May 15, 2016 23:34
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 82.649% when pulling 76d11c0 on mziccard:pubsub-service-options into 1eded7e on GoogleCloudPlatform:pubsub-alpha.

/**
* Shuts down the scheduled executor service if it is no longer used.
*/
void shutdown();

This comment was marked as spam.

This comment was marked as spam.

@aozarov
Copy link
Contributor

aozarov commented May 16, 2016

Conceptually this looks fine too me. Didn't look too carefully at the details.
See my comment regarding the Executor shutdown. Also, did you consider calling it ExecutorFactory
and providing it as a class name (instead of forcing the factory to be serializable)?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 82.743% when pulling 345d702 on mziccard:pubsub-service-options into 1eded7e on GoogleCloudPlatform:pubsub-alpha.

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;

import io.grpc.internal.SharedResourceHolder;

This comment was marked as spam.

@aozarov
Copy link
Contributor

aozarov commented May 16, 2016

Not a thorough review but I like it now!

@mziccard mziccard merged commit fd387a5 into googleapis:pubsub-alpha May 16, 2016
mziccard added a commit to mziccard/gcloud-java that referenced this pull request Jun 27, 2016
…s#1011)

* Add base service option classes for gRPC and HTTP services

* Rename ExecutorProvider to ExecutorFactory, refactor shutdown and serialization
suztomo pushed a commit that referenced this pull request Feb 1, 2023
…o v2.0.1 (#1011)

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.google.api-client:google-api-client-bom](https://togithub.com/googleapis/google-api-java-client/tree/master/google-api-client-bom) ([source](https://togithub.com/googleapis/google-api-java-client)) | `2.0.0` -> `2.0.1` | [![age](https://badges.renovateapi.com/packages/maven/com.google.api-client:google-api-client-bom/2.0.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.api-client:google-api-client-bom/2.0.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.api-client:google-api-client-bom/2.0.1/compatibility-slim/2.0.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.api-client:google-api-client-bom/2.0.1/confidence-slim/2.0.0)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>googleapis/google-api-java-client</summary>

### [`v2.0.1`](https://togithub.com/googleapis/google-api-java-client/blob/HEAD/CHANGELOG.md#&#8203;201-httpsgithubcomgoogleapisgoogle-api-java-clientcomparev200v201-2022-11-05)

[Compare Source](https://togithub.com/googleapis/google-api-java-client/compare/v2.0.0...v2.0.1)

##### Bug Fixes

-   Add error description to batch emptiness validation ([#&#8203;2109](https://togithub.com/googleapis/google-api-java-client/issues/2109)) ([2668dd1](https://togithub.com/googleapis/google-api-java-client/commit/2668dd1348e7710a83e008b1e2b2ff6fceedfedf))
-   **deps:** Update dependency com.google.api-client:google-api-client to v2 ([#&#8203;2108](https://togithub.com/googleapis/google-api-java-client/issues/2108)) ([570a162](https://togithub.com/googleapis/google-api-java-client/commit/570a1625fbb3806961d328d90d784b5f0ed21a0c))
-   **deps:** Update dependency com.google.appengine:appengine-api-1.0-sdk to v2.0.10 ([#&#8203;2174](https://togithub.com/googleapis/google-api-java-client/issues/2174)) ([9077b4a](https://togithub.com/googleapis/google-api-java-client/commit/9077b4ae4c4214cb0fdcb5248f8c7ecbeb51d27f))
-   **deps:** Update dependency com.google.appengine:appengine-api-1.0-sdk to v2.0.6 ([#&#8203;2124](https://togithub.com/googleapis/google-api-java-client/issues/2124)) ([51adc54](https://togithub.com/googleapis/google-api-java-client/commit/51adc541819284dabcdefef39fa39b4a0bd13f6a))
-   **deps:** Update dependency com.google.appengine:appengine-api-1.0-sdk to v2.0.7 ([#&#8203;2131](https://togithub.com/googleapis/google-api-java-client/issues/2131)) ([6892bb2](https://togithub.com/googleapis/google-api-java-client/commit/6892bb293ca578b793fe0024c884b21e675abd45))
-   **deps:** Update dependency com.google.appengine:appengine-api-1.0-sdk to v2.0.8 ([#&#8203;2140](https://togithub.com/googleapis/google-api-java-client/issues/2140)) ([bb6f19c](https://togithub.com/googleapis/google-api-java-client/commit/bb6f19ce2a89f6d419c908eff7faa944ea74799e))
-   **deps:** Update dependency com.google.cloud:libraries-bom to v26.1.0 ([#&#8203;2126](https://togithub.com/googleapis/google-api-java-client/issues/2126)) ([3d0e0ff](https://togithub.com/googleapis/google-api-java-client/commit/3d0e0ff57cde5ca9eb56e5266dc5c37f3777179d))
-   **deps:** Update dependency com.google.cloud:libraries-bom to v26.1.1 ([#&#8203;2134](https://togithub.com/googleapis/google-api-java-client/issues/2134)) ([15ce062](https://togithub.com/googleapis/google-api-java-client/commit/15ce06244a06b64545f145a2ebdfb62863fcbad4))
-   **deps:** Update dependency com.google.cloud:libraries-bom to v26.1.2 ([#&#8203;2143](https://togithub.com/googleapis/google-api-java-client/issues/2143)) ([da2f6f3](https://togithub.com/googleapis/google-api-java-client/commit/da2f6f3e4645ff3b84465943833404526077ad20))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-core).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNy4xIiwidXBkYXRlZEluVmVyIjoiMzQuMTcuMSJ9-->
suztomo pushed a commit that referenced this pull request Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: core api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants