-
Notifications
You must be signed in to change notification settings - Fork 409
feat: Support Not Requiring projectId When Not Required
#1448
Conversation
|
This PR should be followed-up with an issue for removing |
ruyadorno
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 just a small suggestion
| e instanceof Error && | ||
| e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checking for instanceof Error is quite redundant here, if it threw like an error and has a message property like an error, it's def going to be an error 😄
| e instanceof Error && | |
| e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND | |
| e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understandable; the instanceof Error check is more-so a type assertion. By default, TS considers 'e' unknown
| async #getProjectIdOptional(): Promise<string | null> { | ||
| try { | ||
| return await this.getProjectId(); | ||
| } catch (e) { | ||
| if ( | ||
| e instanceof Error && | ||
| e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND | ||
| ) { | ||
| return null; | ||
| } else { | ||
| throw e; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leveraging the Promises API (similar to the previous PR) 😄 might simplify it a bit here again:
| async #getProjectIdOptional(): Promise<string | null> { | |
| try { | |
| return await this.getProjectId(); | |
| } catch (e) { | |
| if ( | |
| e instanceof Error && | |
| e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND | |
| ) { | |
| return null; | |
| } else { | |
| throw e; | |
| } | |
| } | |
| } | |
| #getProjectIdOptional(): Promise<string | null> { | |
| return this.getProjectId() | |
| .catch (e => { | |
| if (e.message === GoogleAuthExceptionMessages.NO_PROJECT_ID_FOUND) { | |
| return null; | |
| } else { | |
| throw e; | |
| } | |
| }) | |
| } |
…ary-nodejs into allow-missing-project-id
🤖 I have created a release *beep* *boop* --- ## [8.5.0](v8.4.0...v8.5.0) (2022-08-31) ### Features * Support Not Requiring `projectId` When Not Required ([#1448](#1448)) ([b37489b](b37489b)) ### Bug Fixes * add hashes to requirements.txt ([#1544](#1544)) ([#1449](#1449)) ([54afa8e](54afa8e)) * remove `projectId` check for `signBlob` calls ([6c04661](6c04661)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [8.5.0](googleapis/google-auth-library-nodejs@v8.4.0...v8.5.0) (2022-08-31) ### Features * Support Not Requiring `projectId` When Not Required ([#1448](googleapis/google-auth-library-nodejs#1448)) ([b37489b](googleapis/google-auth-library-nodejs@b37489b)) ### Bug Fixes * add hashes to requirements.txt ([#1544](googleapis/google-auth-library-nodejs#1544)) ([#1449](googleapis/google-auth-library-nodejs#1449)) ([54afa8e](googleapis/google-auth-library-nodejs@54afa8e)) * remove `projectId` check for `signBlob` calls ([6c04661](googleapis/google-auth-library-nodejs@6c04661)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [8.5.0](googleapis/google-auth-library-nodejs@v8.4.0...v8.5.0) (2022-08-31) ### Features * Support Not Requiring `projectId` When Not Required ([#1448](googleapis/google-auth-library-nodejs#1448)) ([b37489b](googleapis/google-auth-library-nodejs@b37489b)) ### Bug Fixes * add hashes to requirements.txt ([#1544](googleapis/google-auth-library-nodejs#1544)) ([#1449](googleapis/google-auth-library-nodejs#1449)) ([54afa8e](googleapis/google-auth-library-nodejs@54afa8e)) * remove `projectId` check for `signBlob` calls ([6c04661](googleapis/google-auth-library-nodejs@6c04661)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [8.5.0](googleapis/google-auth-library-nodejs@v8.4.0...v8.5.0) (2022-08-31) ### Features * Support Not Requiring `projectId` When Not Required ([#1448](googleapis/google-auth-library-nodejs#1448)) ([b37489b](googleapis/google-auth-library-nodejs@b37489b)) ### Bug Fixes * add hashes to requirements.txt ([#1544](googleapis/google-auth-library-nodejs#1544)) ([#1449](googleapis/google-auth-library-nodejs#1449)) ([54afa8e](googleapis/google-auth-library-nodejs@54afa8e)) * remove `projectId` check for `signBlob` calls ([6c04661](googleapis/google-auth-library-nodejs@6c04661)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [8.5.0](googleapis/google-auth-library-nodejs@v8.4.0...v8.5.0) (2022-08-31) ### Features * Support Not Requiring `projectId` When Not Required ([#1448](googleapis/google-auth-library-nodejs#1448)) ([b37489b](googleapis/google-auth-library-nodejs@b37489b)) ### Bug Fixes * add hashes to requirements.txt ([#1544](googleapis/google-auth-library-nodejs#1544)) ([#1449](googleapis/google-auth-library-nodejs#1449)) ([54afa8e](googleapis/google-auth-library-nodejs@54afa8e)) * remove `projectId` check for `signBlob` calls ([6c04661](googleapis/google-auth-library-nodejs@6c04661)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [8.5.0](googleapis/google-auth-library-nodejs@v8.4.0...v8.5.0) (2022-08-31) ### Features * Support Not Requiring `projectId` When Not Required ([#1448](googleapis/google-auth-library-nodejs#1448)) ([b37489b](googleapis/google-auth-library-nodejs@b37489b)) ### Bug Fixes * add hashes to requirements.txt ([#1544](googleapis/google-auth-library-nodejs#1544)) ([#1449](googleapis/google-auth-library-nodejs#1449)) ([54afa8e](googleapis/google-auth-library-nodejs@54afa8e)) * remove `projectId` check for `signBlob` calls ([6c04661](googleapis/google-auth-library-nodejs@6c04661)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
There are numerous cases where
projectIdisn't required to useGoogleAuth.In fact, I'm failing to find a place where
projectIdis a hard requirement within the library (includingsign, which erroneously requires it and will be removed in this pr: #1447).Fixes #755 🦕