-
Notifications
You must be signed in to change notification settings - Fork 49
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
Conversation
Add reminder to also update revision number in `env/testing/config.json`
Ensures that the index-resource workflow always uses the production config as our Heroku apps default to NODE_ENV=production.¹ ¹ <https://docs.nextstrain.org/projects/nextstrain-dot-org/en/latest/infrastructure.html#environment-or-config-variables>
@@ -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``) |
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 there a situation where the <revision_number>
should differ in these 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.
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?)
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.
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.
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.
Good suggestion! I added in 9d45dd0
Based on feedback from @jameshadfield <#983 (comment)> Ensure that the testing and production configs have matching RESOURCE_INDEX so that we don't accidentally upload an expected resource index version.
Description of proposed changes
Ensures that the index-resource workflow always uses the production config as our Heroku apps default to NODE_ENV=production.¹
¹ https://docs.nextstrain.org/projects/nextstrain-dot-org/en/latest/infrastructure.html#environment-or-config-variables
Related issue(s)
Resolves #980
Checklist