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

Handle trying to play workflows which require upgrade #455

Merged
merged 8 commits into from
Aug 22, 2023

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented May 12, 2023

Sibling: cylc/cylc-flow#5535

Closes #429

  • Intercept error message about versions and upgrading and post
    it to a more helpful error message than stardard "This Cylc Play command failed"
  • Add a toggle for the --upgrade option of Cylc Play

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Tests are included - manually tested, thoughts on automated testing welcomed.
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX. N/A
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

Manual Testing

  • Run a workflow with Cylc 8.1, then stop it.
  • Change to Cylc 8.2
  • Load up the GUI (if you haven't already)
  • Play workflow using the main button - watch for failure
  • Play workflow using the burger bar, edit (pencil) and toggle upgrade (it's near the bottom). watch for success.

@wxtim wxtim added the small label May 12, 2023
@wxtim wxtim added this to the 1.3.0 milestone May 12, 2023
@wxtim wxtim self-assigned this May 12, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2023

Codecov Report

Patch coverage: 33.33% and project coverage change: -0.47% ⚠️

Comparison is base (2b866af) 77.24% compared to head (25a2924) 76.77%.
Report is 2 commits behind head on 1.3.x.

Additional details and impacted files
@@            Coverage Diff             @@
##            1.3.x     #455      +/-   ##
==========================================
- Coverage   77.24%   76.77%   -0.47%     
==========================================
  Files          12       12              
  Lines        1415     1408       -7     
  Branches      239      233       -6     
==========================================
- Hits         1093     1081      -12     
- Misses        276      279       +3     
- Partials       46       48       +2     
Files Changed Coverage Δ
cylc/uiserver/resolvers.py 55.31% <0.00%> (-0.24%) ⬇️
cylc/uiserver/schema.py 71.01% <100.00%> (+0.21%) ⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CHANGES.md Outdated Show resolved Hide resolved
cylc/uiserver/resolvers.py Outdated Show resolved Hide resolved
cylc/uiserver/schema.py Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
cylc/uiserver/resolvers.py Outdated Show resolved Hide resolved
cylc/uiserver/resolvers.py Outdated Show resolved Hide resolved
cylc/uiserver/resolvers.py Outdated Show resolved Hide resolved
@wxtim wxtim requested a review from MetRonnie May 22, 2023 15:16
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

Tested out by artificially changing the cylc_version value to 8.0.3 in workflow_params table, looks good

@MetRonnie MetRonnie linked an issue May 23, 2023 that may be closed by this pull request
@oliver-sanders oliver-sanders modified the milestones: 1.3.0, 1.4.0 Jul 17, 2023
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

Needs rebasing onto master

@wxtim wxtim changed the base branch from 1.2.x to master July 19, 2023 10:48
@wxtim wxtim requested a review from MetRonnie July 19, 2023 10:49
CHANGES.md Show resolved Hide resolved
@wxtim wxtim marked this pull request as draft July 21, 2023 08:39
wxtim and others added 6 commits July 25, 2023 13:42
* Intercept error message about versions and upgrading and post
  it to a more helpful error message than stardard "This Cylc Play command failed"
* Add a toggle for the --upgrade option of Cylc Play
@wxtim wxtim marked this pull request as ready for review July 25, 2023 12:44
@MetRonnie
Copy link
Member

Any reason this can't go on 1.3.x?

Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Dammit, I had a pending review, sorry.

Any reason this can't go on 1.3.x?

Yeah, this could go in 1.3.x, note 1.4.0 will hopefully be released shortly.

cylc/uiserver/schema.py Show resolved Hide resolved
cylc/uiserver/resolvers.py Show resolved Hide resolved
Co-authored-by: Oliver Sanders <[email protected]>
@wxtim
Copy link
Member Author

wxtim commented Aug 22, 2023

@oliver-sanders / @MetRonnie Want to hit the merge button? We don't have separate master/1.4.x branches, so I suppose that we haven't got any major changes?

@MetRonnie MetRonnie changed the base branch from master to 1.3.x August 22, 2023 10:30
@MetRonnie MetRonnie modified the milestones: 1.4.0, 1.3.1 Aug 22, 2023
CHANGES.md Outdated Show resolved Hide resolved
Co-authored-by: Ronnie Dutta <[email protected]>
@wxtim wxtim changed the title Handle trying to play workflows which require upgrade: Improve upgrade warnings Aug 22, 2023
@MetRonnie MetRonnie changed the title Improve upgrade warnings Handle trying to play workflows which require upgrade Aug 22, 2023
@MetRonnie MetRonnie merged commit fae09dd into cylc:1.3.x Aug 22, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

play: add --upgrade option
4 participants