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.

Sign up for GitHub

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

Jump to bottom

cylc vr - restart with no changes? #6261

Open
hjoliver opened this issue Jul 25, 2024 · 5 comments · May be fixed by #6354
Open

cylc vr - restart with no changes? #6261

hjoliver opened this issue Jul 25, 2024 · 5 comments · May be fixed by #6354
Assignees
@oliver-sanders
Labels
could be better Not exactly a bug, but not ideal. small
Milestone
8.3.7

Comments

@hjoliver
Copy link
Member

Validate, reinstall and apply changes to a workflow.

Validate and reinstall a workflow then either:

  • "Reload" the workflow (if it is running),
  • or "play" it (if it is stopped).

Currently, if the workflow is running and no source changes are detected, cylc vr aborts with:

WARNING - No changes to source: No reinstall or reload required.

That's fine, because a reload would achieve absolutely nothing.

However if the workflow is stopped and no source changes are detected, we get:

WARNING - No changes to source: No reinstall or play required.

IMO the "no play required" bit is wrong, it should restart the workflow. I wouldn't do cylc vr if I didn't want the workflow to be running. Maybe I don't recall if if I installed my latest changes already, so I do cylc vr instead of cylc play just in case.
@hjoliver hjoliver added small question Flag this as a question for the next Cylc project meeting. labels Jul 25, 2024
@hjoliver hjoliver added this to the 8.3.x milestone Jul 25, 2024
@hjoliver
Copy link
Member Author

hjoliver commented Jul 25, 2024 •
edited
Loading

Not a big deal since the "workaround" is simply to cylc play if this happens. But still, I think the command logic should be this:

  1. validate source
  2. reinstall changes, if any
  3. reload if running and there are changes, or restart if stopped

@oliver-sanders
Copy link
Member

oliver-sanders commented Jul 31, 2024 •
edited
Loading

I think the command logic should be this

It is, however, it exits at (2) if there are no changes rather than continuing to (3).

Note that you can override this with the --yes option.

@oliver-sanders
Copy link
Member

The purpose of rejecting a reinstallation with no changes is that this likely reflects user error or at least user expectation error (e.g. I forgot to save the file I was editing). So I think this functionality is useful and should remain (we have the --yes option to bypass it).

However, we could consider changing the WARNING into a prompt e.g:

Workflow installation is already up to date (nothing to reinstall).

Would you like to restart the workflow with no changes (y/n)?

(note, for the reload case, the command may as well exit at this point, any reload would most likely be pointless)

Or we could adapt the WARNING to taste, e.g:

Workflow installation is already up to date (nothing to reinstall). Use the "--yes" option if you want to reinstall anyway.

@hjoliver
Copy link
Member Author

hjoliver commented Jul 31, 2024 •
edited
Loading

It is, however, it exits at (2) if there are no changes rather than continuing to (3).

Yes, that's exactly the bit I'm complaining about!

The purpose of rejecting a reinstallation with no changes is that this likely reflects user error or at least user expectation error

That's arguable. On the other side of the coin, as I noted above:

I wouldn't do cylc vr if I didn't want the workflow to be running. Maybe I don't recall if if I installed my latest changes already, so I do cylc vr instead of cylc play just in case.

However,

we could consider changing the WARNING into a prompt e.g:

I think that's acceptable. It allows my use case, but highlights the fact that there are no changes to reinstall first.

@hjoliver
Copy link
Member Author

hjoliver commented Aug 1, 2024

Note that you can override this with the --yes option.

OK I just tested that and it works. I was not aware of it, because it doesn't make sense - there's no interactive prompt to say "yes" to!

--yes                 [reinstall] Skip interactive prompts.

It would make sense with a new prompt as you suggested above.

@oliver-sanders oliver-sanders self-assigned this Sep 3, 2024
@oliver-sanders oliver-sanders added could be better Not exactly a bug, but not ideal. and removed question Flag this as a question for the next Cylc project meeting. labels Sep 3, 2024
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Sep 3, 2024
@oliver-sanders
vr: restart the workflow with no changes according to user prompt
f94a9a9 
* Closes cylc#6261
* If there are no changes to reinstall AND the workflow is stopped,
  prompt the user to see whether they want to restart it anyway.
* This makes `cylc vr` more useful as the "I want to restart my
  workflow" command.
* But it also ensures that they are aware if no changes are present
  as they might have forgotten to press save or run the command on
  the wrong workflow or whatever.
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Sep 3, 2024
@oliver-sanders
vr: restart the workflow with no changes according to user prompt
fdf4c97 
* Closes cylc#6261
* If there are no changes to reinstall AND the workflow is stopped,
  prompt the user to see whether they want to restart it anyway.
* This makes `cylc vr` more useful as the "I want to restart my
  workflow" command.
* But it also ensures that they are aware if no changes are present
  as they might have forgotten to press save or run the command on
  the wrong workflow or whatever.
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Sep 3, 2024
@oliver-sanders
vr: restart the workflow with no changes according to user prompt
b50f4e2 
* Closes cylc#6261
* If there are no changes to reinstall AND the workflow is stopped,
  prompt the user to see whether they want to restart it anyway.
* This makes `cylc vr` more useful as the "I want to restart my
  workflow" command.
* But it also ensures that they are aware if no changes are present
  as they might have forgotten to press save or run the command on
  the wrong workflow or whatever.
@oliver-sanders oliver-sanders added small and removed small labels Sep 3, 2024
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Sep 4, 2024
@oliver-sanders
vr: restart the workflow with no changes according to user prompt
8021ce9 
* Closes cylc#6261
* If there are no changes to reinstall AND the workflow is stopped,
  prompt the user to see whether they want to restart it anyway.
* This makes `cylc vr` more useful as the "I want to restart my
  workflow" command.
* But it also ensures that they are aware if no changes are present
  as they might have forgotten to press save or run the command on
  the wrong workflow or whatever.
@oliver-sanders oliver-sanders modified the milestones: 8.3.x, 8.3.5 Sep 11, 2024
@oliver-sanders oliver-sanders linked a pull request Sep 11, 2024 that will close this issue
Open
8 tasks
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Sep 30, 2024
@oliver-sanders
vr: restart the workflow with no changes according to user prompt
c3230e9 
* Closes cylc#6261
* If there are no changes to reinstall AND the workflow is stopped,
  prompt the user to see whether they want to restart it anyway.
* This makes `cylc vr` more useful as the "I want to restart my
  workflow" command.
* But it also ensures that they are aware if no changes are present
  as they might have forgotten to press save or run the command on
  the wrong workflow or whatever.
@oliver-sanders oliver-sanders modified the milestones: 8.3.5, 8.3.6 Oct 14, 2024
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Oct 22, 2024
@oliver-sanders
vr: restart the workflow with no changes according to user prompt
5af333d 
* Closes cylc#6261
* If there are no changes to reinstall AND the workflow is stopped,
  prompt the user to see whether they want to restart it anyway.
* This makes `cylc vr` more useful as the "I want to restart my
  workflow" command.
* But it also ensures that they are aware if no changes are present
  as they might have forgotten to press save or run the command on
  the wrong workflow or whatever.
@oliver-sanders oliver-sanders modified the milestones: 8.3.6, 8.3.7 Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oliver-sanders oliver-sanders

Labels
could be better Not exactly a bug, but not ideal. small
Projects
None yet
Milestone
8.3.7
Development

Successfully merging a pull request may close this issue.

vr: restart the workflow with no changes according to user prompt oliver-sanders/cylc-flow
2 participants
@hjoliver @oliver-sanders