Skip to content

feat: Add a new ec2 restart option that can also change the instance type#487

Merged
RyanSattlerWork merged 8 commits intomasterfrom
add-restart
Apr 2, 2025
Merged

feat: Add a new ec2 restart option that can also change the instance type#487
RyanSattlerWork merged 8 commits intomasterfrom
add-restart

Conversation

@RyanSattlerWork
Copy link
Collaborator

I've also installed and tested this locally and it seems to work.

@RyanSattlerWork RyanSattlerWork requested a review from tekumara April 1, 2025 04:37
@github-actions github-actions bot added the enhancement New feature or request label Apr 1, 2025
src/aec/main.py Outdated
Cmd(ec2.restart, [
config_arg,
Arg("ident", type=str, help="Name tag of instance or instance id"),
Arg("-t", "--type", type=str, help="New type for the instance")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure if I should use this type of arg here but since the param is optional it felt correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeh makes sense, what about using-m for modify here tho? to align with ec2 modify

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

t feels a little more intuitive here to me but this might be more consistent... I'll change it

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok let's keep it as -t then

src/aec/main.py Outdated
Cmd(ec2.restart, [
config_arg,
Arg("ident", type=str, help="Name tag of instance or instance id"),
Arg("-t", "--type", type=str, help="New type for the instance")
Copy link
Collaborator

Choose a reason for hiding this comment

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

yeh makes sense, what about using-m for modify here tho? to align with ec2 modify

@RyanSattlerWork RyanSattlerWork requested a review from tekumara April 1, 2025 05:58
Copy link
Collaborator

@tekumara tekumara left a comment

Choose a reason for hiding this comment

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

Very handy, thank you!

return describe(config, ident)


def restart(config: Config, ident: str, type: str | None = None, wait_ssm: bool = False) -> list[Instance]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

would you mind adding this to docs/ec2.md and then run make docs to update the usage

@RyanSattlerWork RyanSattlerWork requested a review from tekumara April 1, 2025 23:15
@RyanSattlerWork RyanSattlerWork merged commit 0792b38 into master Apr 2, 2025
2 checks passed
@RyanSattlerWork RyanSattlerWork deleted the add-restart branch April 2, 2025 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants