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

[24.0] Extend regex groups in stdio regex matches #17016

Merged
merged 30 commits into from
Mar 5, 2024
Merged

[24.0] Extend regex groups in stdio regex matches #17016

merged 30 commits into from
Mar 5, 2024

Conversation

bernt-matthias
Copy link
Contributor

@bernt-matthias bernt-matthias commented Nov 13, 2023 •
edited
Loading

Allow to extend groups in the matching regex in the description.

Wondering where to description is shown? Is this broken? The screenshot shows the job info page which shows that the Job Messages are shown as dict and the job details in the history show An error has occurred with this dataset.

Screenshot from 2023-11-13 15-30-29

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@github-actions github-actions bot added this to the 23.2 milestone Nov 13, 2023
@mvdbeek mvdbeek mentioned this pull request Nov 14, 2023
Open
@bernt-matthias bernt-matthias force-pushed the topic/stdio-regex-extend branch 5 times, most recently from 025429a to 7fdf763 Compare November 16, 2023 15:50
mvdbeek
mvdbeek reviewed Nov 16, 2023
View reviewed changes
lib/galaxy/metadata/set_metadata.py Outdated Show resolved Hide resolved
lib/galaxy/metadata/set_metadata.py Outdated Show resolved Hide resolved
@jmchilton
Copy link
Member

The failing test seems relevant. Very exciting work though - this is awesome.

@bgruening
Copy link
Member

Should we use CDTA in the examples?

@bernt-matthias
Copy link
Contributor Author

Thanks @mvdbeek :)

@bgruening CDTA?

@bernt-matthias
Copy link
Contributor Author

I think we can't use CTATA in attributes, isn't it?

This was referenced Dec 9, 2023
Merged
Closed
@mvdbeek mvdbeek modified the milestones: 23.2, 24.0 Dec 19, 2023
@bernt-matthias
Copy link
Contributor Author

@hujambo-dunia could you have a look at the failing client unit test?

@jdavcs
Copy link
Member

jdavcs commented Mar 4, 2024

I think the 2 last commits that touch the JobInformation.vue file are incorrect (e0039f7 and 91eb773): they override the changes introduced in #17566.
Also, the last commit which is empty can be dropped.

@hujambo-dunia
Copy link
Contributor

I think the 2 last commits that touch the JobInformation.vue file are incorrect (e0039f7 and 91eb773): they override the changes introduced in #17566. Also, the last commit which is empty can be dropped.

Agreed - I see what you're saying, including JohnC's commits from 5 days ago getting dropped in my last merge, and will make those adjustments shortly.

@jdavcs
Copy link
Member

jdavcs commented Mar 4, 2024

I'm sorry for being the annoying one here! Here's the thing: those 2 commits I mentioned are still present, and they take the code into a broken state. Even though it's for a short span of 3 commits, fixed in the forth, it is not ideal: the history doesn't tell a coherent story, and if anyone happens to end up on one of those commits (a checkout, a pause in a rebase, etc.), they may be confused.

The fix is simple: reorder the commits, so that those two are followed by the fix. Then squash the three into one. Then you'll have one commit that handles those changes, without any do/undo parts.

bernt-matthias and others added 2 commits March 5, 2024 10:00
@bernt-matthias @hujambo-dunia
make the max discovered files error a "proper" job error message
450c937
@jmchilton @hujambo-dunia
Links to invocations. (by John Chilton via Michelle Savage Vue-compon…
ea359c3 
…ent upgrade)

Upgraded component to Vue 3. Extra space removed.
Remove index enumeration and indentation; separate multiple errors with border line instead. Prevent metadata from printing when there is no value.
Rebased and edited/removed accidental 'merge conflict markers' in file
@mvdbeek mvdbeek changed the base branch from dev to release_24.0 March 5, 2024 15:19
@mvdbeek mvdbeek changed the title Extend regex groups in stdio regex matches [24.0] Extend regex groups in stdio regex matches Mar 5, 2024
@jdavcs
Copy link
Member

jdavcs commented Mar 5, 2024

@hujambo-dunia The rebase look correct - thank you!

@jdavcs @bernt-matthias
Update lib/galaxy/tool_util/xsd/galaxy.xsd
415f419 
Co-authored-by: M Bernt <m.bernt@ufz.de>
jdavcs
jdavcs approved these changes Mar 5, 2024
View reviewed changes
Copy link
Member

@jdavcs jdavcs left a comment

Choose a reason for hiding this comment

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

Looks good to me. Waiting for the tests before merging (there may will be failures that are unrelated). Thanks again for addressing the comments and handling the rebase, @hujambo-dunia!

@hujambo-dunia
Copy link
Contributor

Looks good to me. Waiting for the tests before merging (there may will be failures that are unrelated). Thanks again for addressing the comments and handling the rebase, @hujambo-dunia!

@jdavcs it helps to have such a great teacher!

@jdavcs
Copy link
Member

jdavcs commented Mar 5, 2024

Can you, please, take a look at the client linting error?

@jdavcs jdavcs merged commit 5e05e00 into galaxyproject:release_24.0 Mar 5, 2024
40 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nsoranzo nsoranzo nsoranzo left review comments

@bgruening bgruening bgruening left review comments

@mvdbeek mvdbeek mvdbeek approved these changes

@jdavcs jdavcs jdavcs approved these changes

Assignees
No one assigned
Labels
area/testing area/tool-framework kind/enhancement
Projects
None yet
Milestone
24.0
Development

Successfully merging this pull request may close these issues.
7 participants
@bernt-matthias @jmchilton @bgruening @jdavcs @hujambo-dunia @mvdbeek @nsoranzo