-
Notifications
You must be signed in to change notification settings - Fork 227
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
Pub/Sub retry settings: needs common api between data & admin ops, better docs. #35
Comments
@kir-titievsky do you think this is release blocking? Does this affect the interface or is it something that can be changed post v1.0.0 without necessitating a major version bump? |
I think this is release blocking, unless we are convinced that offering a consistent retry settings API across all operations will not require client library API changes. |
Anything that is passed to the PubSub constructor gets passed to PublisherClient. Has the issue reporter tried the suggestion from the end of the opening post in this thread? (Seems like we are only seeing a piece of the conversation) |
Should we close this one? |
The current release
I receive that message every seconds and it fails every single time (pushed to an HTTP-triggered Google Cloud Functions). No documentation released yet but I dug in the NPM module's code. Any specific format to follow that I got wrong for the gaxOpts.retry ? |
## Version **0.16.0** of [google-proto-files](https://github.com/googleapis/nodejs-proto-files) was just published. <table> <tr> <th align=left> Dependency </th> <td> <code>google-proto-files</code> </td> </tr> <tr> <th align=left> Current Version </th> <td> 0.15.1 </td> </tr> <tr> <th align=left> Type </th> <td> dependency </td> </tr> </table> The version **0.16.0** is **not covered** by your **current version range**. If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update. It might be worth looking into these changes and trying to get this project onto the latest version of google-proto-files. If you have a solid test suite and good coverage, a passing build is a strong indicator that you can take advantage of these changes directly by merging the proposed change into your project. If the build fails or you don’t have such unconditional trust in your tests, this branch is a great starting point for you to work on the update. --- <details> <summary>Release Notes</summary> <strong>v0.16.0</strong> <h2>Features</h2> <ul> <li>(<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="316092585" data-permission-text="Issue title is private" data-url="googleapis/nodejs-proto-files#35" href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/pull/35">#35</a>): Allow passing <code>load()</code> options to protobuf.js. (Thanks, <a class="user-mention" data-hovercard-user-id="583593" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/nmccready">@nmccready</a>!)</li> </ul> </details> <details> <summary>Commits</summary> <p>The new version differs by 19 commits.</p> <ul> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/b4d00bd0b7e6c2d4e0be1395739ded62bd9083e3"><code>b4d00bd</code></a> <code>0.16.0 (#44)</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/47488981af49c1102737e60cad560f9609ac29d2"><code>4748898</code></a> <code>feat: allow load options (#35)</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/91a34aa6ec2fb31932a03f4ed7756210146ea163"><code>91a34aa</code></a> <code>Merge pull request #42 from googleapis/greenkeeper/nyc-12.0.1</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/fca6f7540c45b1ec3916602e1189cef289429908"><code>fca6f75</code></a> <code>Update package.json</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/f0473ada67034a0a294d3f15dd1aa968acb5750a"><code>f0473ad</code></a> <code>chore(package): update nyc to version 12.0.1</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/057563111be9bf9e97e02f893fbed08a9b465db2"><code>0575631</code></a> <code>chore: lock files maintenance (#41)</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/5f38e7f77b6c14a5bbc256acb598fe10f4dbb693"><code>5f38e7f</code></a> <code>chore: lock files maintenance (#38)</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/d077e0b86bf56135301a5240a15c4cbbabf2eb47"><code>d077e0b</code></a> <code>chore: test on node10 (#37)</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/7855ccaca6a1f323719ef2c5a6c77ddd06953cb6"><code>7855cca</code></a> <code>chore: lock files maintenance (#36)</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/e5548189863303d1973d54c5dc23d5eb599fbdaa"><code>e554818</code></a> <code>Merge pull request #32 from googleapis/greenkeeper/sinon-5.0.0</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/0ecbb5e631d5918b8e1cf09e6ab1b6ed6d3baa64"><code>0ecbb5e</code></a> <code>Update package locks</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/838e2d1d84db3e15a0a5a73242ac7399709d6a44"><code>838e2d1</code></a> <code>chore(package): update sinon to version 5.0.0</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/fb18058f58c906ff9e90be80d4f312dbfb9ff68f"><code>fb18058</code></a> <code>chore: workaround for repo-tools EPERM (#33)</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/cf5968e2974b0cecbb6a807ffde696ec37dd6fe6"><code>cf5968e</code></a> <code>chore: setup nighty build in CircleCI (#31)</code></li> <li><a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/commit/c4c973a41ee5810052fb0159ae2f83c18e6f866f"><code>c4c973a</code></a> <code>chore(package): update proxyquire to version 2.0.0 (#30)</code></li> </ul> <p>There are 19 commits in total.</p> <p>See the <a href="https://urls.greenkeeper.io/googleapis/nodejs-proto-files/compare/89183d7d25f9c7e3b9097da820d9d84bd803a92f...b4d00bd0b7e6c2d4e0be1395739ded62bd9083e3">full diff</a></p> </details> <details> <summary>FAQ and help</summary> There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new). </details> --- Your [Greenkeeper](https://greenkeeper.io) bot 🌴
is this still an issue with the latest versions? @callmehiphop |
@kinwa91 I'm actually not 100% what the issue is describing, haha.. In regards to gapics, I can't tell if gax actually uses/caches any call options passed in via the client constructor, but as far as I can tell, it does not. If this is about the hand-written Publisher client, it should be resolved. |
I think as far as providing publishing call options, this has been solved for a while. We could probably improve the client with the steps outlined in googleapis/google-cloud-node#2895, but I'll let @kir-titievsky make the call if that should hold up GA or not. Going to close this issue, but please feel free to re-open if you disagree! |
We should do a thorough review of the API before we go GA. We ought to do that sooner rather than later. I'll bring it up with Kir and crew. |
Copied from original issue: googleapis/google-cloud-node#2788
@kir-titievsky
January 9, 2018 4:42 PM
This is a bug to track progress on an email discussion about setting custom retry settings.
Previous suggestion:
In src/v1/publisher_client.js the gapicConfig contains the contents of the config file you mentioned, and it is passed down to gaxGrpc:
nodejs-pubsub/src/v1/publisher_client.js
Line 166 in 96fd4d6
Here the third parameter, opts.clientConfig, gives configOverrides to gaxGrpc.constructSettings, and, given the name, I guess it might be able to override options.
opts.clientConfig comes from PublisherClient constructor parameter. So I guess you might try settings opts.clientConfig to override any default configuration.
If it does not work for you, please let me know - better with code example - and I'll create an issue for pubsub package.
The text was updated successfully, but these errors were encountered: