NCC-29 Adding fixes so that if a dataset is not fully submitted yet, … - #2
Merged
Conversation
…it can be skipped.
There was a problem hiding this comment.
Pull request overview
This PR improves the DOI minting pipeline’s resilience by treating “not yet submitted” datasets as a normal skip condition (instead of a failure), and by making the AWS deployment/run automation more robust and less secret-dependent.
Changes:
- Introduces
DatasetNotReadyand raises it frommint_doi()when a dataset has no “Submitted” date, enabling callers to skip those datasets. - Updates the CLI runner (
ndbdoi.py) to track/report skipped datasets separately from failures. - Improves AWS automation: preflight validation for required deploy secrets, exports private subnet IDs from CloudFormation, and consumes them in the manual run workflow; simplifies EventBridge → SNS notification templates.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/neotomadoi/neotomaDOI.py |
Raises DatasetNotReady when submission date is missing to skip not-yet-ready datasets. |
src/neotomadoi/exceptions.py |
Adds DatasetNotReady exception type for “expected/transient” skip conditions. |
src/neotomadoi/__init__.py |
Re-exports DatasetNotReady for external callers. |
ndbdoi.py |
Tracks/report skips separately from errors during batch DOI processing. |
infrastructure/doi-minter.yaml |
Simplifies SNS templates and exports private subnet IDs for automation. |
.github/workflows/run-minting.yml |
Uses CloudFormation output for subnets instead of an Actions secret. |
.github/workflows/deploy.yml |
Adds an early step to fail fast when required secrets are missing/empty. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
148
to
149
| else: | ||
| print(f"○ Dataset {dataset_id}: Skipped (already has DOI: {doi_obj.identifiers.get('identifier')})") |
Comment on lines
55
to
60
| echo "CLUSTER=$(get_output ClusterName)" >> $GITHUB_ENV | ||
| echo "TASK_DEF=$(get_output TaskDefinitionArn)" >> $GITHUB_ENV | ||
| echo "TASK_SG=$(get_output TaskSecurityGroupId)" >> $GITHUB_ENV | ||
| echo "TASK_SUBNETS=$(get_output PrivateSubnetIds)" >> $GITHUB_ENV | ||
| echo "LOG_BUCKET=$(get_output LogBucketName)" >> $GITHUB_ENV | ||
| echo "LOG_GROUP=$(get_output LogGroupName)" >> $GITHUB_ENV |
Comment on lines
+534
to
+537
| if not submitted: | ||
| raise DatasetNotReady( | ||
| f"dataset {self.datasetid} has no submission date; not submitted yet" | ||
| ) |
Comment on lines
+530
to
+537
| # No submission date means the owner has not submitted the dataset yet, | ||
| # which is a normal transient state rather than a fault. Raise in both | ||
| # modes, not just `prod`: the sandbox pass is a rehearsal of the mint, so | ||
| # it should skip exactly what production would skip. | ||
| if not submitted: | ||
| raise DatasetNotReady( | ||
| f"dataset {self.datasetid} has no submission date; not submitted yet" | ||
| ) |
This file contains hidden or 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
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.
…it can be skipped.
This pull request introduces several improvements to the DOI minting workflow and codebase, focusing on better handling of not-yet-ready datasets, improved error messaging, and enhanced infrastructure automation. The main changes include introducing a new exception to skip datasets that are not ready for DOI minting, surfacing missing required secrets earlier in the deployment process, simplifying notification messages, and exporting subnet information for better automation.
Error handling and workflow robustness:
DatasetNotReadyexception insrc/neotomadoi/exceptions.pyand integrated it intoneotomaDOI.mint_doito skip datasets that have not been submitted yet (i.e., missing submission date), rather than treating them as failures. This ensures that one not-ready dataset does not block the minting process for others. Updatedndbdoi.pyto track and report skipped datasets separately from errors. [1] [2] [3] [4] [5] [6] [7] [8]Deployment and infrastructure improvements:
.github/workflows/deploy.ymlto check for the presence of all required secrets before deployment, providing clear error messages if any are missing, to prevent opaque failures later in the process.infrastructure/doi-minter.yamlto export the list of private subnet IDs as a stack output, allowing automation to fetch subnets dynamically instead of relying on secrets. Updatedrun-minting.ymlto use this output. [1] [2] [3]Notification simplification:
InputTemplatestrings ininfrastructure/doi-minter.yamlto be single-line messages, avoiding issues with quoted multi-line strings in EventBridge and making notifications clearer. [1] [2]