storage: fix credentials issue. #164
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Connection.prototype.getCredentials = function(callback) { | ||
| var that = this; | ||
| if (this.credentials) { | ||
| process.nextTick(function() { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
stephenplusplus
force-pushed
the
connection
branch
from
15484d5
to
0de5a0d
Compare
| } | ||
| this.fetchToken(function(err) { | ||
| if (err) { | ||
| callback(err); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
stephenplusplus
force-pushed
the
connection
branch
2 times, most recently
from
c47c526
to
25a6b06
Compare
Previously, if a user wanted to generate a signed URL, there was no guarantee a credentials object would have already been generated. By itself, generating a signed URL doesn't require a network connection, thus the uncertainty that we would have already fetched a token. This change requires a valid credentials object to exist before returning the `credentials` object.
stephenplusplus
force-pushed
the
connection
branch
from
25a6b06
to
bb4cd17
Compare
|
Sorry, I'm not familiar with signed URLs. Is there anyone else to review it @silvolu, from Storage team? |
|
Np, this is actually mostly an API change - adding the Currently, we wait until an API request to fetch a token, which makes sense. But to create a signed URL, we don't actually have to make an API call. So, we never get a credentials object (unless the user provided it themselves), which is why I added this method. It will return the credentials if they're available, otherwise it will go build it for us by making a fetchToken call. |
| @@ -95,9 +95,10 @@ module.exports.Token = Token; | |||
| function Connection(opts) { | |||
| events.EventEmitter.call(this); | |||
|
|
|||
| this.opts = opts || {}; | |||
| opts = opts || {}; | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
silvolu
added a commit
that referenced
this pull request
Sep 8, 2014
storage: fix credentials issue.
chingor13
pushed a commit
that referenced
this pull request
Aug 22, 2022
* updated CHANGELOG.md [ci skip] * updated package.json [ci skip] * updated samples/package.json [ci skip]
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Sep 15, 2022
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Oct 12, 2022
sofisl
pushed a commit
that referenced
this pull request
Oct 12, 2022
* build: check dead links on Kokoro * recursive crawl local links * annotate Project#exists with @method * npm run fix
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Oct 13, 2022
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Oct 13, 2022
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Nov 10, 2022
sofisl
pushed a commit
that referenced
this pull request
Nov 10, 2022
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Nov 11, 2022
4 tasks
This was referenced Nov 11, 2022
sofisl
pushed a commit
that referenced
this pull request
Nov 11, 2022
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [jsdoc](https://togithub.com/jsdoc/jsdoc) | [`^3.6.6` -> `^4.0.0`](https://renovatebot.com/diffs/npm/jsdoc/3.6.11/4.0.0) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>jsdoc/jsdoc</summary> ### [`v4.0.0`](https://togithub.com/jsdoc/jsdoc/blob/HEAD/CHANGES.md#​400-November-2022) [Compare Source](https://togithub.com/jsdoc/jsdoc/compare/3.6.11...084218523a7d69fec14a852ce680f374f526af28) - JSDoc releases now use [semantic versioning](https://semver.org/). If JSDoc makes backwards-incompatible changes in the future, the major version will be incremented. - JSDoc no longer uses the [`taffydb`](https://taffydb.com/) package. If your JSDoc template or plugin uses the `taffydb` package, see the [instructions for replacing `taffydb` with `@jsdoc/salty`](https://togithub.com/jsdoc/jsdoc/tree/main/packages/jsdoc-salty#use-salty-in-a-jsdoc-template). - JSDoc now supports Node.js 12.0.0 and later. </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, 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/nodejs-contact-center-insights). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNy4xIiwidXBkYXRlZEluVmVyIjoiMzQuMTcuMSJ9-->
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Nov 11, 2022
- [ ] Regenerate this pull request now. PiperOrigin-RevId: 408439482 Source-Link: googleapis/googleapis@b9f6184 Source-Link: googleapis/googleapis-gen@eb888bc Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZWI4ODhiYzIxNGVmYzdiZjQzYmY0NjM0YjQ3MDI1NDU2NWE2NTlhNSJ9
4 tasks
sofisl
added a commit
that referenced
this pull request
Nov 11, 2022
sofisl
pushed a commit
that referenced
this pull request
Nov 11, 2022
🤖 I have created a release \*beep\* \*boop\* --- ## [2.13.0](https://www.github.com/googleapis/nodejs-dialogflow-cx/compare/v2.12.1...v2.13.0) (2021-08-17) ### Features * add advanced settings for agent level feat: add rollout config, state and failure reason for experiment feat: add insights export settings for security setting feat: add language code for streaming recognition result and flow versions for query p... ([#156](https://www.github.com/googleapis/nodejs-dialogflow-cx/issues/156)) ([80d2f94](https://www.github.com/googleapis/nodejs-dialogflow-cx/commit/80d2f94bcfaa65ea442ee4f8133d850035be04ed)) * added support for DLP templates; expose `Locations` service to get/list avaliable locations of Dialogflow products ([#162](https://www.github.com/googleapis/nodejs-dialogflow-cx/issues/162)) ([b77bde9](https://www.github.com/googleapis/nodejs-dialogflow-cx/commit/b77bde9893cde0c9e766b8e844fa4cca61efb5aa)) ### Bug Fixes * **build:** migrate to using main branch ([#164](https://www.github.com/googleapis/nodejs-dialogflow-cx/issues/164)) ([092d69d](https://www.github.com/googleapis/nodejs-dialogflow-cx/commit/092d69d76716297aff6a671200088d5740b1fd47)) * **deps:** google-gax v2.24.1 ([#165](https://www.github.com/googleapis/nodejs-dialogflow-cx/issues/165)) ([6261c37](https://www.github.com/googleapis/nodejs-dialogflow-cx/commit/6261c37019b3ce94a9876bfa4634837132691888)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Nov 11, 2022
Merged
sofisl
pushed a commit
that referenced
this pull request
Nov 16, 2022
samples: pull in latest typeless bot, clean up some comments Source-Link: https://togithub.com/googleapis/synthtool/commit/0a68e568b6911b60bb6fd452eba4848b176031d8 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:5b05f26103855c3a15433141389c478d1d3fe088fb5d4e3217c4793f6b3f245e
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Nov 16, 2022
This was referenced Nov 16, 2022
sofisl
pushed a commit
that referenced
this pull request
Nov 16, 2022
- [ ] Regenerate this pull request now. Committer: @summer-ji-eng PiperOrigin-RevId: 434859890 Source-Link: googleapis/googleapis@bc2432d Source-Link: googleapis/googleapis-gen@930b673 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiOTMwYjY3MzEwM2U5MjUyM2Y4Y2ZlZDM4ZGVjZDdkM2FmYWU4ZWJlNyJ9
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Nov 30, 2022
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/13e8b547-7af0-436b-b85e-2c1942f8f36a/targets Source-Link: googleapis/synthtool@be74d3e
sofisl
pushed a commit
that referenced
this pull request
Jan 24, 2023
sofisl
pushed a commit
that referenced
this pull request
Jan 25, 2023
sofisl
pushed a commit
that referenced
this pull request
Jan 26, 2023
sofisl
pushed a commit
that referenced
this pull request
Jan 26, 2023
🤖 I have created a release \*beep\* \*boop\* --- ### [1.7.4](https://www.github.com/googleapis/nodejs-ai-platform/compare/v1.7.3...v1.7.4) (2021-07-12) ### Bug Fixes * **deps:** google-gax v2.17.1 ([#164](https://www.github.com/googleapis/nodejs-ai-platform/issues/164)) ([1ec11fc](https://www.github.com/googleapis/nodejs-ai-platform/commit/1ec11fc4fac373ee1a28ac10676fbfe9fc5535d1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
4 tasks
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Sep 13, 2023
4 tasks
sofisl
pushed a commit
that referenced
this pull request
Sep 14, 2023
4 tasks
](https://renovatebot.com){kind=link}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, if a user wanted to generate a signed URL, there was no guarantee a credentials object would have already been generated. By itself, generating a signed URL doesn't require a network connection, thus the uncertainty that we would have already fetched a token. This change requires a valid credentials object to exist before returning the
credentialsobject.This also brings up an issue. We need to go through the code to catch any "maybe async, maybe not" functions, and add in a(#166)process.nextTickwhere necessary.