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

Proposed changes to job-destinations and ansible-galaxy admin tutorials to prevent ansible-playbook errors. #4308

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Proposed changes to job-destinations and ansible-galaxy admin tutorials to prevent ansible-playbook errors. #4308

merged 2 commits into from
Nov 2, 2023

Conversation

Edmontosaurus
Copy link
Contributor

Following the job-destinations and ansible-galaxy instructions as they currently are, will both result in failed ansible playbook runs. See the details about the problems and my proposed fixed below. If my proposed fixes are not ideal, then feel free to reject them and just use this pull-request as informational messages about current problems in these tutorials:

  • ansible-galaxy tutorial: Following this tutorial will result in ansible playbook errors at line 926 where the variable "miniconda_version: 4.12.0" is defined. This tutorial used to work without errors but recently the miniconda role fails when miniconda_version 4.12 is defined. The cause of this is because the miniconda role by default attempts to install the latest conda version and then downgrades to the version defined in "miniconda_version". The latest miniconda versions can't be downgraded to 4.12 because a 'UnsatisfiableError' error will occur because of python version constraints. (the latest conda version being installed by the role uses python 3.11 which causes the version constraint error.) One workaround for this problem is to define extra variables, as can be seen in the diff. Two other possible solutions are to have the playbook install the latest conda version or to modify the code of the miniconda_role.

  • job-destinations tutorial: In this tutorial, the student is instructed to run the ansible playbook after having added the usegalaxy_eu.tpv_auto_lint role. However running this playbook at this point will also result in ansible errors. This happens because TPV itself has not yet been installed. Skipping the playbook run at this point and only running it after the "Configuring TPV" section proved to be a workaround for me. As mentioned in the tutorial, adding TPV as a runner will cause Galaxy to automatically install TPV. The tpv_auto_lint role will only work at this point because it requires TPV to be installed. My proposed fix would be to remove the instruction to run the ansible-playbook at this point and run it after the "Configuring TPV" section.

Thank you for contributing to the GTN! 💛

Please check that:

  • Your pull request has a good title, e.g.
    • "Fix typo in ansible-galaxy tutorial"
    • "Add new transcriptomics tutorial covering a new sequencing technology"
  • Please replace this entire pull request message with a description of your changes, it will help us review your pull request faster
  • Check that your images are allowed to be re-hosted by the GTN

Once these are done, you're ready to go!

Consider sustainable computing with Draft Mode 💚

Is your pull request finished? 100% ready to be merged? Or do you want some time to work on it first (which we encourage! It's great to have visibility)

If not, then please consider creating this pull request as a draft.

Regular pull requests will trigger automated test runs when they are created
and every time you update them.

As a small contribution to sustainable computing, we skip the most energy intensive of these tests for pull requests marked as drafts. Once your pull request is ready to go, then you can click a button saying "Ready for Review" and the tests will be run!

Following the job-destinations and ansible-galaxy instructions as they currently are, will both result in failed ansible playbook runs. See the details about the problems and my proposed fixed below. If my proposed fixes are not ideal, then feel free to reject them and just use this pull-request as informational messages about current problems in these tutorials:

* ansible-galaxy tutorial: Following this tutorial will result in ansible playbook errors at line 926 where the variable "miniconda_version: 4.12.0" is defined. This tutorial used to work without errors but recently the miniconda role fails when miniconda_version 4.12 is defined.
The cause of this is because the miniconda role by default attempts to install the latest conda version and then downgrades to the version defined in "miniconda_version". The latest miniconda versions can't be downgraded to 4.12 because a 'UnsatisfiableError' error will occur because of python version constraints. (the latest conda version being installed by the role uses python 3.11 which causes the version constraint error.) One workaround for this problem is to define extra variables, as can be seen in the diff. Two other possible solutions are to have the playbook install the latest conda version or to modify the code of the miniconda_role.

* job-destinations tutorial: In this tutorial, the student is instructed to run the ansible playbook after having added the usegalaxy_eu.tpv_auto_lint role. However running this playbook at this point will also result in ansible errors. This happens because TPV itself has not yet been installed. Skipping the playbook run at this point and only running it after the "Configuring TPV" section proved to be a workaround for me. As mentioned in the tutorial, adding TPV as a runner will cause Galaxy to automatically install TPV. The tpv_auto_lint role will only work at this point because it requires TPV to be installed. My proposed fix would be to remove the instruction to run the ansible-playbook at this point and run it after the "Configuring TPV" section.
@hexylena
Copy link
Member

hexylena commented Oct 3, 2023

Hi @Edmontosaurus, I'm sorry we didn't get around to reviewing this earlier (it came in during GCC month, just before I went into vacation.)

Welcome to the GTN! Thank you for the very detailed bug report @Edmontosaurus ! Would you please add your username name to the testers of these tutorials? (And to CONTRIBUTORS.yaml of course.)

As of #4370 we've chosen to bump to a newer conda version which should resolve part of this issue, I've merged in a commit to fix the conflict, but once you record your contributions I'll merge this!

Copy link
Collaborator

@cat-bro cat-bro left a comment

Choose a reason for hiding this comment

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

This change for the TPV tutorial looks great. The tutorial is broken without it. Very keen to merge. Thank you @Edmontosaurus !

@hexylena
Copy link
Member

hexylena commented Nov 2, 2023

since @Edmontosaurus hasn't responded here or in slack we can just merge.

@hexylena hexylena merged commit 6d73a70 into galaxyproject:main Nov 2, 2023
4 checks passed
@Edmontosaurus
Copy link
Contributor Author

Edmontosaurus commented Nov 2, 2023

Hi! Thanks for merging. I've been meaning to respond, but only saw I got some responses to this recently and have been very busy. Apologies for my lack of response :)

@hexylena
Copy link
Member

hexylena commented Nov 2, 2023

@Edmontosaurus do you want to provide some contributor information? and we will add you to the contributors of this tutorial. You can provide any of the following fields, 'joined' of course is set automatically

martenson:
    name: Martin Čech
    email: [email protected]
    matrix: 'martenson:matrix.org'
    orcid: 0000-0002-9318-1781
    elixir_node: cz
    contact_for_training: true
    location:
      country: USA
      lat: 37.0
      lon: -122.0
    fediverse: https://mastodon.world/@martenson
    fediverse_flavor: mastodon
    joined: 2017-09

@Edmontosaurus
Copy link
Contributor Author

@Edmontosaurus do you want to provide some contributor information? and we will add you to the contributors of this tutorial. You can provide any of the following fields, 'joined' of course is set automatically

martenson:
    name: Martin Čech
    email: [email protected]
    matrix: 'martenson:matrix.org'
    orcid: 0000-0002-9318-1781
    elixir_node: cz
    contact_for_training: true
    location:
      country: USA
      lat: 37.0
      lon: -122.0
    fediverse: https://mastodon.world/@martenson
    fediverse_flavor: mastodon
    joined: 2017-09

Yes, no problem. This would be:

edmontosaurus:
    name: Edwin den Haas
    matrix: 'edmontosaurus:matrix.org'

Would you like me to create an additional pull-request for this? Or can you add this to the CONTRIBUTORS.yaml file yourself?

@hexylena
Copy link
Member

hexylena commented Nov 2, 2023

I'll take care of it, no worries.

@hexylena
Copy link
Member

hexylena commented Nov 2, 2023

#4477 PR is here :)

@Edmontosaurus
Copy link
Contributor Author

Cool. Thanks :)

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.

3 participants