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

Rule - `st2ctl reload` Reloads all except rules #2861

Closed
vaishalipavashe opened this Issue Aug 10, 2016 · 13 comments

Comments

Projects
None yet
5 participants
@vaishalipavashe

vaishalipavashe commented Aug 10, 2016

  • Intervaltimer rule was created and tested. Tried disabling it by updating rule yaml file and reloaded st2ctl as st2ctl reload but still rule is enabled:

image

  • From st2ctl reload output found that it reloads all except rules. Performed st2ctl reload --register-rules to reload rules and now it shows disabled:

image

@armab

This comment has been minimized.

Member

armab commented Aug 10, 2016

Yeah, you have to reload with --register-rules manually.

This is something that Manas described previously in #1619 (comment)

The reason is that different packs can contain default rules and it's safer for administrator not to load them.
If all rules are registered - then you have many triggers enabled, running different actions (which most of us probably don't want for first st2 install).

@Kami will correct me if I'm wrong.

@armab

This comment has been minimized.

Member

armab commented Aug 10, 2016

But I think there is definitely a user usability problem here, since it's something that raised many times (the behavior of reload is not obvious).

The good approach for me feels like:

  • pack install should load all content within a pack (and not load everything else, out of current pack scope)
  • there should be ability to (re)load files for specific pack (isolated)
  • make the interaction interface easy for users/predictable/obvious
@samirsss

This comment has been minimized.

Member

samirsss commented Aug 10, 2016

Isn't it the users responsibility to make sure that the rules they want to not trigger be saved in disabled state?

I think this ask is a fair thing. I'm getting used to the system and I didn't realize that the 1 rule I had written up didn't get reloaded when I did st2ctl reload. (it wasn't an important rule) but still the expectation that I had was rules would get reloaded.

@lakshmi-kannan @Kami

@Kami

This comment has been minimized.

Member

Kami commented Aug 10, 2016

Yeah the idea behind rules being disabled by default was that unlike actions, rules are usually not totally generic and they need to be modified / customized by the user before they can be used.

That's why we decided not to register rules by default - if user didn't modify the rule or disable it, they would get surprised.

@samirsss

This comment has been minimized.

Member

samirsss commented Aug 10, 2016

Then should we close this PR? I dont have the history on this and hence just thinking from users perspective.

@Kami

This comment has been minimized.

Member

Kami commented Aug 10, 2016

We can all think about it some more about re-evaluate our decision aka if we still think that's the best default behavior.

/cc @lakshmi-kannan

@lakshmi-kannan

This comment has been minimized.

Contributor

lakshmi-kannan commented Aug 10, 2016

I think arma nailed it here: #2861 (comment)

st2ctl reload IMO should register rules by default. This has caused user frustration. We just have to make sure we don't ship dumb rules in enabled state with the product. If users place their rules on disk, it becomes their responsibility.

I thought manas added st2 rule enable/disable. st2 rule list --disabled and st2 rule list --enabled are things we can build for better visibility.

@Kami

This comment has been minimized.

Member

Kami commented Aug 10, 2016

We just have to make sure we don't ship dumb rules in enabled state with the product. If users place their rules on disk, it becomes their responsibility.

Agreed.

@samirsss

This comment has been minimized.

Member

samirsss commented Aug 10, 2016

So in summary we think its good to register rules as part of st2ctl reload.

Yes there is a --enabled --disabled flag in the st2 rule list command.

@vaishalipavashe

This comment has been minimized.

vaishalipavashe commented Aug 10, 2016

Rules from examples pack can be disabled by default as they got loaded when I did st2ctl reload —register-rules for 1st time. I had to delete them manually and disable.

@Kami

This comment has been minimized.

Member

Kami commented Aug 11, 2016

it's good that we have re-evaluated that decision.

@samirsss if you feel like it you can make the change so rules are registered by default and everything which goes along with making a change (code changes, changelog entry, documentation entry, upgrade notes entry, etc.).

@samirsss

This comment has been minimized.

Member

samirsss commented Aug 11, 2016

Yep i can do it...

@lakshmi-kannan

This comment has been minimized.

Contributor

lakshmi-kannan commented Aug 15, 2016

Seems like the fix went it. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment