-
Notifications
You must be signed in to change notification settings - Fork 125
Fix migrate to store expanded globs in the new state #4068
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
Conversation
| Building python_artifact... | ||
| update jobs.sample_job | ||
|
|
||
| Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged |
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.
It has 1 to change here. The intent is to make this 0 right?
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 catch, looking into this.
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.
We already have change even in terraform due to dynamic_version: #4074
There is also separate (pre-existing) issue with num_workers, will look into this in a follow up.
|
Commit: 26c3dd2
22 interesting tests: 13 flaky, 7 KNOWN, 2 SKIP
Top 49 slowest tests (at least 2 minutes):
|
pietern
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.
Can you include a "Badness"?
14cfe65 to
676a867
Compare
676a867 to
2a743aa
Compare
|
Commit: b4f3df3
36 interesting tests: 25 flaky, 7 KNOWN, 3 RECOVERED, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
## Why Terraform only adds it if neither autoscale nor num_workers are not set. It filters it out if autoscale is set. ## Tests New acceptance test that has different configurations and asserts that requests are the same. Difference also disappears in existing tests: in default-python recorded requests now have no difference wrt num_workers. In migrate/default-python num_workers no longer triggers drift post migration #4068
Since migrate includes phases.Build but not phases.DeployPrepare, it did not process globs.
I moved globs processing to phases.Build, they are logically related to artifact building.