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

Feat: stop sending external_id for env0_aws_credentials resource #593

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

TomerHeber
Copy link
Collaborator

Issue & Steps to Reproduce / Feature Request

fixes #591

Solution

  1. Removed external_id from all the locations I found,
  2. Updated unit tests, examples, acceptance tests, and integration tests.

@@ -23,13 +23,6 @@ func resourceCostCredentials(providerName string) *schema.Resource {
ForceNew: true,
Required: true,
},
"external_id": {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed also from cost credentials...
I was guessing this also needs to be removed. I hope this was correct.
(Worst case, I'll add it back).

@yaronya
Copy link
Contributor

yaronya commented Feb 20, 2023

@TomerHeber is this ready for review?

@avnerenv0
Copy link
Contributor

@yaronya it doesn't matter.

@TomerHeber sorry but we've had some blowback from our customers regarding this sudden change.
We'll need to make the external_id optional as a first deprecation step - with some wording in the docs that "it will be removed entirely in the near future".
Can you do that here? Or open another PR if you prefer?

Thanks

@TomerHeber
Copy link
Collaborator Author

@yaronya it doesn't matter.

@TomerHeber sorry but we've had some blowback from our customers regarding this sudden change. We'll need to make the external_id optional as a first deprecation step - with some wording in the docs that "it will be removed entirely in the near future". Can you do that here? Or open another PR if you prefer?

Thanks

@yaronya @avnerenv0 - closing this PR (can re-open in the future).
I will add a deprecated message instead.

@avnerenv0
Copy link
Contributor

deprecation period is over - we want this removed

@avnerenv0 avnerenv0 reopened this Mar 12, 2023
@avnerenv0
Copy link
Contributor

@TomerHeber the API is going to ignore external_id from now on and we wanna remove it from the provider - can you resolve the conflicts here please?

Copy link
Contributor

@avnerenv0 avnerenv0 left a comment

Choose a reason for hiding this comment

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

please resolve the conflicts - other than that - ready to merge 👍

@avnerenv0
Copy link
Contributor

@Wassap124 Wassap124 merged commit 8375f9c into main Mar 13, 2023
@Wassap124 Wassap124 deleted the feat-remove-external_id-#591 branch March 13, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop sending external_id for env0_aws_credentials resource
4 participants