[BUG][NRPTI-1165] token refresh for core importer #1247
Conversation
| @@ -66,7 +66,7 @@ Moving the **App** from `dev` (`:latest`) to `test` | |||
| ```sh | |||
| oc project f00029-tools | |||
| oc tag nrpti:test nrpti:test-backup | |||
| oc tag nrpti:lastest nrpti:test | |||
| oc tag nrpti:latest nrpti:test | |||
| ``` | |||
There was a problem hiding this comment.
unrelated typo fix
| return payload.access_token; | ||
| } | ||
| return payload; | ||
| }; |
There was a problem hiding this comment.
changed this to return the whole object because it includes the ttl for the token
| @@ -43,7 +43,8 @@ class CoreDataSource { | |||
|
|
|||
| try { | |||
| // Get a new API access token. | |||
| this.client_token = await getCoreAccessToken(CORE_CLIENT_ID, CORE_CLIENT_SECRET, CORE_GRANT_TYPE); | |||
| const apiAccess = await getCoreAccessToken(CORE_CLIENT_ID, CORE_CLIENT_SECRET, CORE_GRANT_TYPE); | |||
| this.client_token = apiAccess.access_token; | |||
|
|
|||
There was a problem hiding this comment.
refactored due to the change below
There was a problem hiding this comment.
Don't missing migration files cause errors to run during migration steps, if this was already merged and run by the databases at different levels? It might depend on the tool, but I know NRPTI has a migrations table in it.
There was a problem hiding this comment.
yeah. this migration was never run on prod
| * | ||
| * @memberof CoreDocumentsDataSource | ||
| */ | ||
| async setClientToken() { |
There was a problem hiding this comment.
I remember for a different but similar issue we had to request a longer token expiry time, maybe it was more specific to a users credentials. graceful solution :)
| isAPITokenExpired(expiryTime) { | ||
| return Date.now() >= expiryTime; | ||
| } |
There was a problem hiding this comment.
As an edge case, would you want to artificially increase Date.now() to account for time for the API request to be made?
There was a problem hiding this comment.
there's a TIME_BUFFER subtracted from the expiryTime. we request a refresh 30 seconds before we actually need it to allow for any delays
| await this.taskAuditRecord.updateTaskRecord({ itemTotal: this.status.itemTotal }); | ||
|
|
||
| const permitUtils = new PermitUtils(this.auth_payload, RECORD_TYPE.Permit); | ||
|
|
||
| // Push records to proccess into a Promise array to be processed in parallel. | ||
| for (let i = 0; i < permits.length; i += jobCount) { | ||
| for (let i = 0; i < numPermits; i += jobCount) { |
There was a problem hiding this comment.
@@L78-L89
This could be a good opportunity to pull this code out into a function and write a test for it. This kind of code can be confusing to read.
Not a blocker, I know we didn't write this code we are just updating the permit length reference.
|
* unrelated typo fix * refactored getCoreAccessToken to return whole data obj instead of just the token * refactoring to account for a changed return type in supporting function * added a check for expired api token to the core-documents importer * fixed typo * removed migration to update bcmi collections as it is not needed.
* unrelated typo fix * refactored getCoreAccessToken to return whole data obj instead of just the token * refactoring to account for a changed return type in supporting function * added a check for expired api token to the core-documents importer * fixed typo * removed migration to update bcmi collections as it is not needed.
Description
The API access token granted when doing the core-documents import is only valid for 900 sec (15 minutes). While this is not normally noticeable when only importing a couple documents, an import of all documents can take over an hour to do.
This PR includes the following proposed change(s):