-
Notifications
You must be signed in to change notification settings - Fork 16
Use AIRFLOW_ENV=development on .development.env file
#4439
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
base: main
Are you sure you want to change the base?
Conversation
lauriemerrell
left a comment
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.
One comment, otherwise LGTM. I had made the fix in dags.py but I can just rebase on this
lauriemerrell
left a comment
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.
Ope, missed a save -- here's the comment
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.
As noted in the ticket, I am just not sure what this env var does in this context, not sure if there's been a functional issue on the test archiver?
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.
Yeah, I haven't work with that yet. I asked about your tests to see if any were related to this on.
We definitely need to test it before merging this change.
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.
@lauriemerrell I reviewed my previous changes, and I think they were wrong.
The only place that looks like we would need to change is on this .development.env file.
Can you test your test archiver with this change?
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.
@erikamov I think you and I went in opposite directions -- here, I went in the direction of having the code check for the new value: github.com/cal-itp/data-infra/commit/df5a106421e2ddadd3fe4a35129e7b7042fd6b17. I guess either way is fine, I wasn't sure if changing it back to development would cause other problems either
I don't have a local archiver instance so not sure best way to test that
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 try to set something up but not sure of overhead)
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.
Basically I discovered this issue on #4350 and I resolved it for that use case by just checking for the updated value, I didn't want to scope creep on that PR so wrote this ticket, but I do not have a handle on the other places that variable is checked
6a4543e to
d5897b2
Compare
AIRFLOW_ENV to match correct valuesAIRFLOW_ENV=development on development.env file
|
Terraform plan in iac/cal-itp-data-infra-staging/composer/us No changes. Your infrastructure matches the configuration.📝 Plan generated in Plan Terraform for Warehouse and DAG changes #1108 |
AIRFLOW_ENV=development on development.env fileAIRFLOW_ENV=development on .development.env file
|
Terraform plan in iac/cal-itp-data-infra-staging/airflow/us Plan: 0 to add, 2 to change, 0 to destroy.Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
!~ update in-place
Terraform will perform the following actions:
# google_storage_bucket_object.calitp-staging-composer-catalog will be updated in-place
!~ resource "google_storage_bucket_object" "calitp-staging-composer-catalog" {
!~ content = (sensitive value)
!~ crc32c = "2FXNxw==" -> (known after apply)
!~ detect_md5hash = "bE9f/PjreIWPig75/NS6Vw==" -> "different hash"
!~ generation = 1764122637987817 -> (known after apply)
id = "calitp-staging-composer-data/warehouse/target/catalog.json"
!~ md5hash = "bE9f/PjreIWPig75/NS6Vw==" -> (known after apply)
name = "data/warehouse/target/catalog.json"
# (16 unchanged attributes hidden)
}
# google_storage_bucket_object.calitp-staging-composer-manifest will be updated in-place
!~ resource "google_storage_bucket_object" "calitp-staging-composer-manifest" {
!~ content = (sensitive value)
!~ crc32c = "5pENHQ==" -> (known after apply)
!~ detect_md5hash = "3KrGaUvOP8W/6eVlzvOsOw==" -> "different hash"
!~ generation = 1764122639162554 -> (known after apply)
id = "calitp-staging-composer-data/warehouse/target/manifest.json"
!~ md5hash = "3KrGaUvOP8W/6eVlzvOsOw==" -> (known after apply)
name = "data/warehouse/target/manifest.json"
# (16 unchanged attributes hidden)
}
Plan: 0 to add, 2 to change, 0 to destroy.📝 Plan generated in Plan Terraform for Warehouse and DAG changes #1108 |
d5897b2 to
1dc3cc2
Compare
|
Terraform plan in iac/cal-itp-data-infra/airflow/us Plan: 0 to add, 2 to change, 0 to destroy.Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
!~ update in-place
Terraform will perform the following actions:
# google_storage_bucket_object.calitp-composer-catalog will be updated in-place
!~ resource "google_storage_bucket_object" "calitp-composer-catalog" {
!~ content = (sensitive value)
!~ crc32c = "00iJ/w==" -> (known after apply)
!~ detect_md5hash = "yfkZas/eBQMAqfz2eXnuDw==" -> "different hash"
!~ generation = 1764100054963994 -> (known after apply)
id = "calitp-composer-data/warehouse/target/catalog.json"
!~ md5hash = "yfkZas/eBQMAqfz2eXnuDw==" -> (known after apply)
name = "data/warehouse/target/catalog.json"
# (16 unchanged attributes hidden)
}
# google_storage_bucket_object.calitp-composer-manifest will be updated in-place
!~ resource "google_storage_bucket_object" "calitp-composer-manifest" {
!~ content = (sensitive value)
!~ crc32c = "S9xfig==" -> (known after apply)
!~ detect_md5hash = "GEAgWaMaB3xMcxPCFeJXSg==" -> "different hash"
!~ generation = 1764100056657852 -> (known after apply)
id = "calitp-composer-data/warehouse/target/manifest.json"
!~ md5hash = "GEAgWaMaB3xMcxPCFeJXSg==" -> (known after apply)
name = "data/warehouse/target/manifest.json"
# (16 unchanged attributes hidden)
}
Plan: 0 to add, 2 to change, 0 to destroy.📝 Plan generated in Plan Terraform for Warehouse and DAG changes #1108 |
Description
Based on researching the past commits, the only place that may need to be changed is on
.development.envfile. Since there was not a staging environment when it was created, there are tests only for two options:developmentorcal-itp-data-infra.This PR sets
AIRFLOW_ENV=developmenton.development.env file.Resolves [#4426]
Type of change
How has this been tested?
Post-merge follow-ups