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

Pass many 'register' components for 'packs.install', 'packs.load', 'st2ctl reload' #1619 #1624

Merged
merged 3 commits into from
Jun 16, 2015
Merged

Pass many 'register' components for 'packs.install', 'packs.load', 'st2ctl reload' #1619 #1624

merged 3 commits into from
Jun 16, 2015

Conversation

arm4b
Copy link
Member

@arm4b arm4b commented Jun 12, 2015

Problem

One of the problems described in #1619 is that packs.install reloads only actions by default, bringing problems (especially in ChatOps) when installed pack contains aliases or sensors:

st2 run packs.install packs=st2-google repo_url=jfryman/st2-google

Solution

This PR brings possibility to pass several register components.
If not provided register=actions,aliases,sensors is default value.

# before (1 component only):
st2 run packs.load register=actions

# now:
st2 run packs.install packs=hubot register=aliases,rules
st2 run packs.load register=rules,sensors

which would trigger reload with many arguments (only 1 was accepted before):

st2ctl reload --register-aliases --register-rules

armab added 2 commits June 12, 2015 18:02
st2ctl reload --register-sensors

st2ctl reload --register-sensors --register-rules

st2ctl reload
st2ctl reload --verbose
…1619

#
st2 run packs.load register=rules,sensors
st2 run packs.install packs=hubot register=aliases,rules

# by default registers 'actions,aliases,sensors'
st2 run packs.load
st2 run packs.install packs=hubot
@arm4b arm4b self-assigned this Jun 12, 2015
if [ -z ${2} ]; then
REGISTER_FLAGS="--register-sensors --register-actions --register-aliases"
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this is a lot better. Hated this monstrosity :P

Copy link
Member

Choose a reason for hiding this comment

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

I'm just slightly worried, since I don't remember if we have any end to end tests for st2ctl.

If we don't, now it would be a good time to add some to st2tests repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

st2tests looks like some kind of acceptance tests, pretty cool thing. I'll do it a bit later.

@Kami StackStorm/st2tests#16

@manasdk
Copy link
Contributor

manasdk commented Jun 12, 2015

This works for me. We can sort out the array pieces laters.

The only drawback with string are the type is that actions, sensors, aliases is not the same as actions,sensors,aliases. You could write a trim filter for jinja if motivated to support both

@arm4b
Copy link
Member Author

arm4b commented Jun 12, 2015

actions, sensors, aliases

Good point,
I've checked st2 run packs.install packs="hubot, ansible" is parsed well, so I would like to include trim logic here for consistency.

@arm4b
Copy link
Member Author

arm4b commented Jun 12, 2015

@manasdk when you'll have time take a look at that #1231 (comment) if it's possible to do it in right way.

Splitting that string into array is absolutely dirty hackery. If possible, I would be happy to use an array there.

@arm4b
Copy link
Member Author

arm4b commented Jun 15, 2015

@Kami if this PR looks ok for you, can you double-check and merge it?
Or should I include st2tests first?

@Kami
Copy link
Member

Kami commented Jun 16, 2015

Sorry for the delay. I will go ahead, test it and if everything looks OK, merge it.

Tests are not a blocker right now, but having them asap would be great :)

Kami added a commit that referenced this pull request Jun 16, 2015
Pass many 'register' components for 'packs.install', 'packs.load', 'st2ctl reload' #1619
@Kami Kami merged commit 892af91 into StackStorm:master Jun 16, 2015
@Kami
Copy link
Member

Kami commented Jun 16, 2015

Merged into master - changes look and work fine.

Thanks.

@arm4b
Copy link
Member Author

arm4b commented Jun 16, 2015

Thanks!

@arm4b arm4b deleted the 1619-fix-action-register branch June 16, 2015 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants