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

Removed st2web dependency on st2 module #279

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nmaludy
Copy link
Member

@nmaludy nmaludy commented Dec 22, 2020

The StackStorm.st2web role incorrectly declares a dependency on the StackStorm.st2 module. This causes issues if trying to use these roles to only install individual components on a system. Example: if you only want to install nginx and st2web on a box, it also pulls in st2.

@nmaludy nmaludy added the bug label Dec 22, 2020
@nmaludy nmaludy self-assigned this Dec 22, 2020
@pull-request-size pull-request-size bot added the size/XS PR that changes 0-9 lines. Quick fix/merge. label Dec 22, 2020
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Thanks! This enhancement makes sense to me 👍

I left one comment to address.

roles/StackStorm.st2web/defaults/main.yml Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/S PR that changes 10-29 lines. Very easy to review. and removed size/XS PR that changes 0-9 lines. Quick fix/merge. labels Dec 23, 2020
@nmaludy nmaludy requested a review from arm4b December 23, 2020 13:47
@@ -48,6 +48,7 @@ Below is the list of variables you can redefine in your playbook to customize st
| `st2_packs` | `[ st2 ]` | List of packs to install. This flag does not work with a `--python3` only pack.
| `st2_python_packages` | `[ ]` | List of python packages to install into the `/opt/stackstorm/st2` virtualenv. This is needed when deploying alternative auth or coordination backends which depend on Python modules to make them work.
| **st2web**
| `st2web_version` | `latest` | `st2web` version to install. `present` to install available package, `latest` to get automatic updates, or pin it to numeric version like `2.2.0`.
Copy link
Member

@arm4b arm4b Dec 23, 2020

Choose a reason for hiding this comment

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

Can you please also list the st2web_revision var in the README as well? This looks like oversight from the past.

@@ -10,3 +10,6 @@ st2web_ssl_certificate_key: null

# String with a custom nginx configuration file to replace st2.conf. If not provided, the default st2.conf will be used.
st2web_nginx_config: null

# StackStorm version to install. `present` to install available package, `latest` to get automatic updates or pin it to numeric version like `2.2.0`.
st2web_version: 'latest'
Copy link
Member

@arm4b arm4b Dec 23, 2020

Choose a reason for hiding this comment

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

Please move the st2web_version variable added before the st2web_revision above to make them logically aligned as we have in other Ansible roles like st2 and st2chatops.

@arm4b
Copy link
Member

arm4b commented Jan 11, 2021

@nmaludy Could you please provide those minor changes and so we could merge the PR?

@CLAassistant
Copy link

CLAassistant commented May 11, 2022

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug size/S PR that changes 10-29 lines. Very easy to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants