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 best practice in README.md #211

Merged
merged 3 commits into from
Apr 16, 2024
Merged

Conversation

dometto
Copy link
Contributor

@dometto dometto commented Apr 15, 2024

Hi all,

I tried following the best practices guide in the README in the playbook I'm working on and noticed I had to deviate from it at some points. Maybe I did something wrong, but wanted to open this PR in case the best practice indeed needs to be updated. I'm pretty sure about the following changes:

  • update to the postgres_objects role name
  • supervisorctl should not be used

I needed to add the systemd post_tasks to actually get Galaxy to start up the first time, but maybe there's a better way for this?

README.md Outdated
state: restarted
listen: restart galaxy
- name: Galaxy gravity restart
command: "/usr/local/bin/galaxyctl graceful"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
command: "/usr/local/bin/galaxyctl graceful"
command: "{{ galaxy_gravity_command }} graceful"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this sadly doesn't work because galaxy_gravity_command is a default within the galaxy role, so it's not available in the playbook. But {{ galaxy_venv_dir }}/bin/galaxyctl might be a bit better?

Copy link
Member

Choose a reason for hiding this comment

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

Ah I knew I was using that based on seeing it from somewhere: the built in handlers We have two handlers defined, that call this already (handlers/gravity_23.0.yml)

So these handlers as defined in the example are no longer necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right! Those handlers weren't working for me, but I now see that may be due to #202 -- so probably that issue should be fixed, and the handlers should not be defined in the Readme. :)

README.md Outdated
register: galaxy_status

- name: Galaxy gravity start
command: "/usr/local/bin/galaxyctl start"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
command: "/usr/local/bin/galaxyctl start"
command: "{{ galaxy_gravity_command }} start"

@hexylena
Copy link
Member

in case the best practice indeed needs to be updated.

thank you for opening this, yes, they do. We haven't been keeping the readme up to date, not many people are reaching this repository via the readme and trying to setup galaxy with just that information. Most folks go through the course where we document the 'best practices' more thoroughly.

@dometto dometto changed the title Update best pracrtice in README.md Update best practice in README.md Apr 16, 2024
hexylena and others added 2 commits April 16, 2024 13:58
Add extra needed apt dependencies to best practices playbook.
@hexylena hexylena merged commit 6d24332 into galaxyproject:main Apr 16, 2024
@dometto dometto deleted the patch-1 branch April 16, 2024 15:15
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.

2 participants