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

Update for the ena upload tool: added ISA json support, changed phrasings, moved macro's,... #5802

Merged
merged 32 commits into from
Mar 23, 2024

Conversation

B0r1sD
Copy link
Contributor

@B0r1sD B0r1sD commented Feb 29, 2024

FOR CONTRIBUTOR:

  • - I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • - License permits unrestricted use (educational + commercial)
  • - This PR adds a new tool or tool collection
  • - This PR updates an existing tool or tool collection
  • - This PR does something else (explain below)

@B0r1sD B0r1sD marked this pull request as ready for review March 15, 2024 11:14
@bedroesb
Copy link
Contributor

@bgruening This pull request brings some improvements to the interface + adds the new feature that was brought to ENA-upload-cli, ISA-JSON support. I think we can close #5691 .

@bedroesb
Copy link
Contributor

Thanks @B0r1sD for all the improvements!

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Cool! Looks great! One comment.

<assert_contents>
<has_n_lines n="4"/>
<has_n_columns n="17"/>
<!-- <has_line_matching expression="alias\tstudy_alias\tsample_alias\tlibrary_name\ttitle\taccession\tsubmission date\tstatus\tdesign_description\tlibrary_source\tlibrary_strategy\tlibrary_selection\tlibrary_layout\tinsert_size\tplatform\tinstrument_model\tsubmission_date"/> -->
Copy link
Member

Choose a reason for hiding this comment

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

should this go in again? some more assertions?

Copy link
Contributor

Choose a reason for hiding this comment

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

We had some problems with letting it match for some weird reason. Although we where using the exact same "correct" output as matching expression, the test kept complaining :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I was able to bring 3 out of 4 back in without problems!

tools/ena_upload/ena_upload.xml Show resolved Hide resolved
tools/ena_upload/ena_upload.xml Outdated Show resolved Hide resolved
tools/ena_upload/ena_upload.xml Outdated Show resolved Hide resolved
@bedroesb
Copy link
Contributor

Ok I think the feedback is implemented, let me know if we should improve something else!

@bedroesb
Copy link
Contributor

@bgruening Anything that should still be done?

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Ready from my side? @bernt-matthias?

@bernt-matthias bernt-matthias merged commit c7f54e6 into galaxyproject:main Mar 23, 2024
11 checks passed
@gxydevbot
Copy link
Collaborator

Attention: deployment failure!

https://github.com/galaxyproject/tools-iuc/actions/runs/8404730682

@bernt-matthias
Copy link
Contributor

Restarted

@bgruening
Copy link
Member

Thanks :)

@bedroesb
Copy link
Contributor

@bgruening or @bernt-matthias thank a lot for merging this! How long does it take on average for a tool to be available in the toolshed?

@bernt-matthias
Copy link
Contributor

Should be already there.

@bedroesb
Copy link
Contributor

https://toolshed.g2.bx.psu.edu/repository?repository_id=0db04aa13ef9d2f8 shows me that there is a more resent revision but the dropdown itself does not visualize it, and neither does it in de galaxy instance if I have to believe our admins .. :/

@bernt-matthias
Copy link
Contributor

Hmm. Odd. All looks good (also here) except for the dropdown.

@bedroesb
Copy link
Contributor

Weirdly the latest tool that has been updated on this repo compleasm, did get a correct new version in the toolshed. So it is a problem specific to this one. Could the failed action have caused a problem? and should I trigger a ne version?

@bernt-matthias
Copy link
Contributor

Can you report the issue here: https://github.com/galaxyproject/usegalaxy-playbook/issues and ask if bumping might be an option?

@mvdbeek
Copy link
Member

mvdbeek commented Mar 27, 2024

What do you mean by bumping ? The usegalaxy playbook is for deploying playbooks, that's not a good place to report anything. Here or the galaxy repo are good places.

@mvdbeek
Copy link
Member

mvdbeek commented Mar 27, 2024

Looks like the first upload timed out but succeeded without creating the repository metadata, the second upload didn't do anything. @bernt-matthias try uploading manually with some whitespace change ?

@bernt-matthias
Copy link
Contributor

I updated manually.

@bedroesb
Copy link
Contributor

Thanks a lot everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants