-
Notifications
You must be signed in to change notification settings - Fork 107
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
Update the queue-granules task to fetch collection configs for each granule from S3 #284
Conversation
This makes it match the other protocol mixins.
CHANGELOG.md
Outdated
@@ -6,6 +6,23 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. | |||
|
|||
## [Unreleased] | |||
|
|||
### Changed | |||
- The **queue-granules** task no longer takes a "collection" in its config. |
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.
Add the ticket number to these notes please. Also this would be a breaking change, correct?
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.
Maybe we should put all these changes in bullets under one line item and call out what changes will need to be made to the workflow configuration to support this.
packages/ingest/parse-pdr.js
Outdated
// get all the file specs in each group | ||
const specs = fileGroup.objects('FILE_SPEC'); | ||
// FIXME This is a very generic error | ||
if (specs.length === 0) throw new Error(); |
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.
Probably should at least put a message in the error?
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.
And test the error somehow
packages/ingest/parse-pdr.js
Outdated
|
||
if (!dataType) throw new PDRParsingError('DATA_TYPE is missing'); | ||
const files = specs.map(parseSpec.bind(null, pdrName)); | ||
const granuleId = extractGranuleId(files[0].name, granuleIdExtraction); |
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.
Is it possible files would be empty here? Is that something we should check for?
packages/ingest/parse-pdr.js
Outdated
|
||
// Get all the file groups | ||
const fileGroups = parsed.objects('FILE_GROUP'); | ||
function granuleFromFileGroup(fileGroup, granuleIdExtraction, pdrName) { |
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.
This function needs docs.
packages/ingest/sftp.js
Outdated
const Crypto = require('./crypto').DefaultProvider; | ||
const log = require('@cumulus/common/log'); |
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.
Shouldn't this be const { log } = require('@cumulus/common')'
?
tasks/queue-granules/index.js
Outdated
return [dataType, JSON.parse(collectionConfig)]; | ||
} | ||
|
||
async function fetchCollectionConfigs(stackName, Bucket, dataTypes) { // eslint-disable-line require-jsdoc, max-len |
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.
Document this function
tasks/queue-granules/tests/index.js
Outdated
recursivelyDeleteS3Bucket(t.context.templateBucket), | ||
sqs().deleteQueue({ QueueUrl: t.context.event.config.queueUrl }).promise() | ||
]); | ||
}); | ||
|
||
function uploadCollectionConfig(testContext, dataType, collectionConfig) { |
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.
document
packages/ingest/parse-pdr.js
Outdated
// Get all the file groups | ||
const fileGroups = parsed.objects('FILE_GROUP'); | ||
function granuleFromFileGroup(fileGroup, granuleIdExtraction, pdrName) { | ||
if (!fileGroup.get('DATA_TYPE')) throw new PDRParsingError('DATA_TYPE is missing'); |
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.
Can we test this error?
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.
The workflow needs to be updated.
https://github.com/cumulus-nasa/cumulus-integration-tests/blob/master/workflows.yml
Also, in the integration test, it would be better to have a PDR which contains granules from different datatypes.
packages/ingest/parse-pdr.js
Outdated
|
||
dataType = dataType.value; | ||
const files = specs.map(parseSpec.bind(null, pdrName)); | ||
const granuleId = extractGranuleId(files[0].name, granuleIdExtraction); |
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.
Should we throw an error on empty files?
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.
I've created CUMULUS-507 to handle that.
…are present in PDR
await S3.put(process.env.internal, key, JSON.stringify(item)); | ||
const collectionConfigStore = | ||
new CollectionConfigStore(process.env.internal, process.env.stackName); | ||
await collectionConfigStore.put(item.name, item); |
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.
We now have one central piece of code that manages the storage and retrieval of collection configs in S3.
Summary: Update the queue-granules task to fetch collection configs for each granule from S3
Addresses CUMULUS-450: Restore functionality to get correct collection for IngestGranule
Changes
findTmpTestDataDirectory
function from@cumulus/common/test-utils
.sftpMixin
instead of using a default export. This matches the mixins for the other protocols.parsePdr()
function.Test Plan
Things that should succeed before merging.
./bin/eslint-ratchet
and verify that eslint errors have not increased.eslint-ratchet-high-water-mark
if the score has improved