Skip to content
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

Always use production config.json in index-resources GH Action #983

Merged
merged 3 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ jobs:
- run: npm ci
- run: npm run lint:server
- run: npm run lint:static-site
- run: node ./scripts/check-resource-index-match.js

# Build into Heroku slug so we can deploy the same build that we test.
build:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/index-resources.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ jobs:
env:
HEROKU_TOKEN: ${{ secrets.HEROKU_TOKEN_READ_PROTECTED }}
IS_WORKFLOW_CALL: ${{ inputs.is-workflow-call || false }}
# Ensure that we are using the production configs to match Heroku apps
CONFIG_FILE: ./env/production/config.json
run: |
# Assign to variable before output to ensure error exit code propagates to GH Action job
ref_matrix=$(./scripts/get-resource-index-ref-matrix)
Expand Down
3 changes: 2 additions & 1 deletion docs/resource-collection.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ The nextstrain.org testing & production configs currently set this to
``s3://nextstrain-inventories/resources/v<revision_number>.json.gz``.

If you make any updates that changes the structure or the contents of the resource
index JSON, then bump the ``<revision_number>`` within the configs (``env/production/config.json``)
index JSON, then bump the ``<revision_number>`` within the configs
(``env/testing/config.json`` and ``env/production/config.json``)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a situation where the <revision_number> should differ in these files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, env/testing/config.json is only used for local testing right now so I don't think so?
(Maybe if we ever set up a completely separate testing instance on AWS there would be reason for them to differ?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - in this case I continue to think that we should add a check in CI that they are the same. Otherwise it's too easy to overlook the docs, get things working locally, and then overwrite the production version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion! I added in 9d45dd0

so that any uploads to S3 does not disrupt the production server.

These updates include changes to:
Expand Down
16 changes: 16 additions & 0 deletions scripts/check-resource-index-match.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { readFileSync } from "fs";

const PRODUCTION_CONFIG = "env/production/config.json";
const TESTING_CONFIG = "env/testing/config.json";
const RESOURCE_INDEX = "RESOURCE_INDEX";

const production_index = JSON.parse(readFileSync(PRODUCTION_CONFIG, 'utf8'))[RESOURCE_INDEX]
const testing_index = JSON.parse(readFileSync(TESTING_CONFIG, 'utf8'))[RESOURCE_INDEX]

if (production_index !== testing_index) {
console.error(`ERROR: Please make sure ${PRODUCTION_CONFIG} and ${TESTING_CONFIG} have the same ${RESOURCE_INDEX}!"`)
process.exit(1);
} else {
console.log(`Confirmed ${PRODUCTION_CONFIG} and ${TESTING_CONFIG} have the same ${RESOURCE_INDEX}`);
process.exit(0);
}