Approach for changing old-format report_ids to new format #2376
Conversation
This file contains 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
|
Terraform plan for meta No changes. Your infrastructure matches the configuration.✅ Plan applied in Deploy to Development and Management Environment #259 |
|
We tested this locally and on a subset of records on |
|
Terraform plan for dev No changes. Your infrastructure matches the configuration.✅ Plan applied in Deploy to Development and Management Environment #259 |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 5a53adb |
timoballard
approved these changes
Oct 4, 2023
The old parameter has been deprecated, and there appears to be stronger requirements for a URL, not just a hostname, for the endpoint value.
The newer argument referred to in the deprecation warning (`endpoints.s3`) is not being accepted. So we'll go back to just specifying `endpoint`, but give it a URL instead of just a hostname for the endpoint.
This file populates the necessary Terraform config for local development (an alternative to the GHA workflow, which supplies the backend vars as secrets). This change accounts for the stricter checking on the `endpoint`, which should be a URL rather than a hostname; see the previous commit for more context.
Similar to previous commits: We have to provide a URL here, since Terraform got stricter about the value it wants for "endpoint". This commit is for the workflow used during the actual deployment.
danswick
added a commit
that referenced
this pull request
Oct 4, 2023
) * Approach for changing old-format report_ids to new format. * Change data_source. * Configure Terraform's s3 backend endpoint with a URL The old parameter has been deprecated, and there appears to be stronger requirements for a URL, not just a hostname, for the endpoint value. * Second try at giving the Terraform S3 backend a URL endpoint The newer argument referred to in the deprecation warning (`endpoints.s3`) is not being accepted. So we'll go back to just specifying `endpoint`, but give it a URL instead of just a hostname for the endpoint. * Propagate S3 backend endpoint URL change to local config This file populates the necessary Terraform config for local development (an alternative to the GHA workflow, which supplies the backend vars as secrets). This change accounts for the stricter checking on the `endpoint`, which should be a URL rather than a hostname; see the previous commit for more context. * Provide URL for s3 backend endpoint during Terraform apply Similar to previous commits: We have to provide a URL here, since Terraform got stricter about the value it wants for "endpoint". This commit is for the workflow used during the actual deployment. --------- Co-authored-by: Tadhg O'Higgins <2626258+tadhg-ohiggins@users.noreply.github.com> Co-authored-by: Bret Mogilefsky <bret.mogilefsky@gsa.gov>
asteel-gsa
pushed a commit
that referenced
this pull request
Oct 5, 2023
* Approach for changing old-format report_ids to new format. * Change data_source. * Configure Terraform's s3 backend endpoint with a URL The old parameter has been deprecated, and there appears to be stronger requirements for a URL, not just a hostname, for the endpoint value. * Second try at giving the Terraform S3 backend a URL endpoint The newer argument referred to in the deprecation warning (`endpoints.s3`) is not being accepted. So we'll go back to just specifying `endpoint`, but give it a URL instead of just a hostname for the endpoint. * Propagate S3 backend endpoint URL change to local config This file populates the necessary Terraform config for local development (an alternative to the GHA workflow, which supplies the backend vars as secrets). This change accounts for the stricter checking on the `endpoint`, which should be a URL rather than a hostname; see the previous commit for more context. * Provide URL for s3 backend endpoint during Terraform apply Similar to previous commits: We have to provide a URL here, since Terraform got stricter about the value it wants for "endpoint". This commit is for the workflow used during the actual deployment. --------- Co-authored-by: Bret Mogilefsky <bret.mogilefsky@gsa.gov>
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.
Bad
report_idsContaminate the DB—
Force them to conform.
As part of #2327, change the existing in-DB
report_idvalues to conform to #2326.The best way to test this locally is if you have old-format
report_ids in your local environment and you can see them get changed to the new format after checking out this branch and running the Django management command.Components:
update_oldformat_reportids, which does what it says and also writes a record of the changes to a text file. This is fragile, but probably enough for our purposes..profileto run this command on startup.SingleAuditChecklist.saveto allow us to make this change without running intoLateChangeError.PR checklist: submitters
maininto your branch shortly before creating the PR. (You should also be mergingmaininto your branch regularly during development.)PR checklist: reviewers
make docker-clean; make docker-first-run && docker compose up; then rundocker compose exec web /bin/bash -c "python manage.py test"The larger the PR, the stricter we should be about these points.