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

resource/job: remove attribute allocation_ids #357

Merged
merged 3 commits into from
Jul 26, 2023
Merged

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented Jul 26, 2023

Storing the list of allocation IDs in a nomad_job resource can lead to ever-changing plans since the list can change outsite of Terraform.

Retrieving this type of information is better suited for a data source.

The Terraform provider deprecation guide recommends attributes to be first marked as deprecated before full removal in the next major release.

Given that the next release will be a major release, this commit introduces a breaking change in behaviour (allocation IDs are not fetched by default anymore) but provides a fallback with the read_allocation_ids attribute.

Both attributes are marked as deprecated and will be removed in the next major release.

nomad_allocations data source added in #358

Storing the list of allocation IDs in a `nomad_job` resource can lead to
ever-changing plans since the list can change outsite of Terraform.

Retrieving this type of information is better suited for a data source.

The Terraform provider deprecation guide recommends attributes to be
first marked as deprecated before full removal in the next major
release.

Given that the next release will be a major release, this commit
introduces a breaking change in behaviour (allocation IDs are not
fetched by default anymore) but provides a fallback with the
`read_allocation_ids` attribute.

Both attributes are marked as deprecated and will be removed in the next
major release.
@lgfa29 lgfa29 requested review from angrycub and gulducat July 26, 2023 19:27
This was referenced Jul 26, 2023
Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

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

I like the availability of the fallback for now. I assume you've tested it out of band, since the only test code changes are removals?

@lgfa29
Copy link
Contributor Author

lgfa29 commented Jul 26, 2023

Yup. This option causes several tests to be flaky and, since it will be removed in the future, I didn't think it was worth to keep a flaky test around.

@lgfa29 lgfa29 merged commit 355efe5 into main Jul 26, 2023
2 of 3 checks passed
@lgfa29 lgfa29 deleted the f-job-remove-alloc-ids branch July 26, 2023 20:14
@lgfa29 lgfa29 added this to the 2.0.0 milestone Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants