robinovitch61 / wander

A terminal app/TUI for HashiCorp Nomad
MIT License
442 stars 17 forks source link

Restart jobs, tasks and allocs #122

Closed cshintov closed 9 months ago

cshintov commented 9 months ago

Implements https://github.com/robinovitch61/wander/issues/120

Implemented:

robinovitch61 commented 9 months ago

Thanks for starting work on this @cshintov ! Check out some of my thoughts thoughts here on overall design for admin stuff and LMK what you think.

For this PR, consider incorporating some of that design - it would be cool if the "admin task" table had just a single entry for now, "Restart Job/Allocation " by the end of this work. You can do jobs and allocations/tasks separately over different PRs if you'd like.

For each PR, target the next branch, as that's where new feature development should live.

cshintov commented 9 months ago

Continuing here.

Thanks for these thoughts. I like the X -> admin menu idea. Makes it explicit and cautions the users. But I think the typing yes to do the action, a little tedious in a scenario like @attachmentgenie got into. I like the k9s way. Opening a pop up window, press tab to cancel or ok buttons, default can be cancel, and press enter to do the action.

If we can implement the pop up window nicely, that sounds great!

Looked into how this can be implemented. Not natively supported by lip gloss. It's in the pipeline.

There's a work around though! https://github.com/charmbracelet/lipgloss/pull/102#issuecomment-1375875184

I think we can postpone this and go with your idea of typing yes for now. What do you think? Should I try to implement the pop up window like mentioned in the above comment instead?

robinovitch61 commented 9 months ago

Continuing here.

Thanks for these thoughts. I like the X -> admin menu idea. Makes it explicit and cautions the users.

But I think the typing yes to do the action, a little tedious in a scenario like @attachmentgenie got into. I like the k9s way. Opening a pop up window, press tab to cancel or ok buttons, default can be cancel, and press enter to do the action.

If we can implement the pop up window nicely, that sounds great!

Looked into how this can be implemented. Not natively supported by lip gloss. It's in the pipeline.

There's a work around though!

https://github.com/charmbracelet/lipgloss/pull/102#issuecomment-1375875184

I think we can postpone this and go with your idea of typing yes for now. What do you think? Should I try to implement the pop up window like mentioned in the above comment instead?

I'm happy either way! If the workaround is too complicated and/or doesn't handle edge cases well (eg changing the terminal size while the popup is open breaks things), let's stick with the "type yes" screen for now. If user expediency is a concern, we can accept just typing "y" too.

cshintov commented 9 months ago

Btw, we can't restart a job. I Checked the api code. There's no such function. Probably why the UI also doesn't provide a button for this.

When I want to restart a job, I usually add a dummy meta block to simulate/trigger a job restart. Maybe we can do that here as well.

robinovitch61 commented 9 months ago

Btw, we can't restart a job. I Checked the api code. There's no such function. Probably why the UI also doesn't provide a button for this.

When I want to restart a job, I usually add a dummy meta block to simulate/trigger a job restart. Maybe we can do that here as well.

Interesting! Could we issue a stop, poll for when it's stopped, then issue a start? I'm afk so can't look at the nomad api right now.

Don't feel you have to implement more than one admin action in this first PR. I would pick the one that's easiest and add just that to the list of admin actions for now.

cshintov commented 9 months ago

Have implemented just restarting tasks for now, although Admin Menu lists others as well. Will remove.

The UI is not very nice, but does the job.

wander-restart-task.webm

robinovitch61 commented 9 months ago

Have implemented just restarting tasks for now, although Admin Menu lists others as well. Will remove.

The UI is not very nice, but does the job.

wander-restart-task.webm

Nice! I'll give the code a review this weekend :)

robinovitch61 commented 9 months ago

Remember to modify GetPageKeyHelp to include the help text for X admin. I think we'll want it to the right of ctrl+w toggle wrap on the second row of the relevant pages.

robinovitch61 commented 9 months ago

@cshintov I played around a bit here: https://github.com/robinovitch61/wander/pull/124

check it out and steal whatever you'd like to from there into your work here (I don't plan on merging my PR, it's just for sharing purposes)

cshintov commented 9 months ago

@cshintov I played around a bit here: #124

check it out and steal whatever you'd like to from there into your work here (I don't plan on merging my PR, it's just for sharing purposes)

Thanks, that looks more clean, so I've stolen it all 😂!

I've added the X -> Admin help and returning FailureMsg as you suggested.

Admin Help and Failure Messages

robinovitch61 commented 9 months ago

@cshintov I played around a bit here: #124 check it out and steal whatever you'd like to from there into your work here (I don't plan on merging my PR, it's just for sharing purposes)

Thanks, that looks more clean, so I've stolen it all 😂!

I've added the X -> Admin help and returning FailureMsg as you suggested.

Screenshare.-.2024-01-28.11.10.00.PM.webm

Looks great! Will give this a final review today

cshintov commented 9 months ago

Made changes according to the review. Think it's ready now.

but feel free to merge whenever you feel good about it.

But I don't have permission to merge!