Ecarton/cumulus 3751 pg update #3916
Conversation
etcart
changed the base branch from
release-18.5.x
to
ecarton/cumulus-3751-s3-task
| ingestExecutionArn = await buildAndStartWorkflow( | ||
| stackName, | ||
| config.bucket, | ||
| 'IngestAndPublishGranuleWithOrca', |
There was a problem hiding this comment.
why this workflow: 'IngestAndPublishGranuleWithOrca',? how does that play into this test/task?
| ); | ||
| }); | ||
|
|
||
| describe('under normal circumstances', () => { |
There was a problem hiding this comment.
NIT: more descriptive title for this describe
| //upload to cumulus | ||
| try { | ||
| const { $metadata, Payload } = await lambda().send(new InvokeCommand({ | ||
| FunctionName: `${stackName}-ChangeGranuleCollectionS3`, |
There was a problem hiding this comment.
assuming this needs to be ran first before the PG change? can this be moved to the beforeAll or is it better to just have it here 🤔 since the test is meant to test PG
There was a problem hiding this comment.
overall the test looks great 🙌
| @@ -0,0 +1,2 @@ | |||
| /nyc.config.js | |||
There was a problem hiding this comment.
NIT: should /src/ be included in this file?
| const sinon = require('sinon'); | ||
| const { createSnsTopic } = require('@cumulus/aws-client/SNS'); | ||
| const { generateLocalTestDb, migrationDir, GranulePgModel, CollectionPgModel, translateApiCollectionToPostgresCollection, translateApiGranuleToPostgresGranule, localStackConnectionEnv } = require('@cumulus/db'); | ||
|
|
There was a problem hiding this comment.
for a db test like this, do you not need this stuff:
process.env.AccessTokensTable = randomId('token');
process.env.stackName = randomId('stackname');
process.env.system_bucket = randomId('system-bucket');
process.env.TOKEN_SECRET = randomId('secret');
process.env.backgroundQueueUrl = randomId('backgroundQueueUrl');
| ); | ||
| pgRecords.granules = await granuleModel.insert( | ||
| knex, | ||
| await Promise.all(granules.map(async (g) => ( |
There was a problem hiding this comment.
NIT: naming from g -> granule
| t.context.esIndex = esIndex; | ||
| t.context.esClient = esClient; | ||
|
|
||
| t.context.knexAdmin = knexAdmin; |
There was a problem hiding this comment.
is all this bucket stuff needed just for this? what is it doing in this case?
| "sampleFileName": "BROWSE.MOD11A1.A2017200.h19v04.006.2017201090724.hdf", | ||
| "bucket": "private" | ||
| }, | ||
| { |
There was a problem hiding this comment.
do you need this for unit tests, why not just use the fakeFactories stuff instead, unless there is a deliberate need I'm not understanding (bc its task/workflow....etc.)?
| uploadTestDataToBucket(config.bucket, s3data, sourceGranulePath), | ||
| addCollections(stackName, config.bucket, collectionsDir, testSuffix, testId), | ||
| addCollections(stackName, config.bucket, targetCollectionsDir, testSuffix, testId), | ||
| addProviders(stackName, config.bucket, providersDir, config.bucket, testSuffix), |
There was a problem hiding this comment.
why would you need providers?
Summary: adds Postgres update task to change granules over to new pg details for new collection
Addresses CUMULUS-3751: Develop amazing new feature
Changes
(still need to) add to workflow integration test to include this
PR Checklist