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

Support execute command configuration in ECS cluster config #25

Merged
merged 5 commits into from
Nov 7, 2023

Conversation

hanscg
Copy link
Contributor

@hanscg hanscg commented Nov 2, 2023

No description provided.

@hanscg hanscg self-assigned this Nov 2, 2023
Copy link
Contributor

@FurqanHabibi FurqanHabibi left a comment

Choose a reason for hiding this comment

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

LGTM, once all checks are successful.

@hanscg
Copy link
Contributor Author

hanscg commented Nov 2, 2023

Validation failed because optional was not a thing in Terraform 0.13 :orz:
Do I need to modify it to support 0.13 or is it acceptable to raise the minimum version to 0.14?

@FurqanHabibi
Copy link
Contributor

Personally it's fine to increase to 0.14, @michaelaw320 ?

@michaelaw320
Copy link
Collaborator

I think it's fine to update minimum supported version to 0.14

@hanscg
Copy link
Contributor Author

hanscg commented Nov 2, 2023

@michaelaw320 Do we need to bump major version?

@michaelaw320
Copy link
Collaborator

yes, let's bump it major

@hanscg hanscg force-pushed the execute-command-config branch from b6fe21e to 350a99d Compare November 6, 2023 05:52
@hanscg
Copy link
Contributor Author

hanscg commented Nov 6, 2023

optional was experimental until Terraform 1.3 :/
I think pushing the min version up to 1.3 is a bit too much, let me see if I can avoid optional.

@hanscg hanscg force-pushed the execute-command-config branch from 350a99d to 38b5fd2 Compare November 6, 2023 07:09
@hanscg
Copy link
Contributor Author

hanscg commented Nov 6, 2023

Avoided optional by just using any 😬
Bumped modules in the examples and also the minimum Terraform version to 1.0
Will bump major version.

@FurqanHabibi
Copy link
Contributor

the minimum Terraform version to 1.0

What required the bump to 1.0?

@hanscg
Copy link
Contributor Author

hanscg commented Nov 6, 2023

What required the bump to 1.0?

The upgraded module in examples requires Terraform 1.0.
I upgraded the modules because the old versions are not compatible with AWS provider version 5.
I couldn't find a middle ground - module version compatible with provider v5 but also compatible with Terraform 0.13.

Considering 1.0 has been around for 2 years I think it's time to bump the version too.

Copy link
Contributor

@FurqanHabibi FurqanHabibi left a comment

Choose a reason for hiding this comment

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

LGTM, awesome work 💪

@hanscg
Copy link
Contributor Author

hanscg commented Nov 7, 2023

@michaelaw320 Any objection to merging this?

Copy link
Collaborator

@michaelaw320 michaelaw320 left a comment

Choose a reason for hiding this comment

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

LGTM, please merge and create new release

@hanscg hanscg merged commit 1e42d45 into HENNGE:main Nov 7, 2023
23 checks passed
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.

3 participants