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

Feature request: more obvious warning when attempting to action a disabled config #1266

Closed
reidsunderland opened this issue Oct 18, 2024 · 8 comments
Labels
enhancement New feature or request Sugar nice to have features... not super important.

Comments

@reidsunderland
Copy link
Member

I vaguely recall discussing this, but I couldn't find an issue for it.

It would be nice to have a more obvious warning when trying to start/stop/cleanup/etc a disabled config.

$ sr3 status | grep disa
sarra/get_ssc_wmo_bulletins                          disa    3/3     0   0%   0%     0    0.00s    0.00s        0  0.0%       0B/s       0m/s       0B/s       0B/s

$ sr3 start sarra/get_ssc_wmo_bulletins
starting:( 0 ) Done

$ sr3 cleanup sarra/get_ssc_wmo_bulletins
cleanup: 2024-10-18 20:26:46,031 1664 [INFO] sarracenia.moth.amqp getCleanUp deleting queue q_feeder.sarra.get_ssc_wmo_bulletins
2024-10-18 20:26:46,037 1664 [INFO] sarracenia.flowcb.log on_cleanup hello
removing state file: /local/home/sarra/.cache/sr3/sarra/get_ssc_wmo_bulletins/sarra.get_ssc_wmo_bulletins.feeder.qname
removing state file: /local/home/sarra/.cache/sr3/sarra/get_ssc_wmo_bulletins/subscriptions.json

@robjarawan

@reidsunderland reidsunderland added enhancement New feature or request Sugar nice to have features... not super important. labels Oct 18, 2024
@petersilva
Copy link
Contributor

perhaps previous discussiuon was around #1159 ...

@reidsunderland
Copy link
Member Author

Yes, that's what I remembered!

@petersilva
Copy link
Contributor

@mshak2 maybe this is a good one for you...
I think the implementation is:

  • look in sarracenia/sr.py start() routine.
  • test self.configs[c][cfg]['status'] ... == 'disabled' ...
  • Do something smart... "print a message?"

options:

  • could just check each one and print a message for each disable one saying no can do... or
  • could loop through all the filtered_configs and then refuse to do anything if any of them are disabled?
  • or do the loop, but do it anyway if dangerWillRobinson?

dunno... would have to play...

@petersilva
Copy link
Contributor

when no set is specified (to do all of them...) we don't want it to not start any...
(can start anything on our dev machines now, because some configs are disabled.)

@petersilva
Copy link
Contributor

  • people are thinking just having the message is good enough.
  • or could have an override --wololo or danger... but for should not be needed for "normal" stuff.
  • there is a self._action_all_configs which is set to differentiate between selection, and just everything... likely when all_configs active, it should ignore the disabled (not even print a message.)

@petersilva
Copy link
Contributor

petersilva commented Oct 30, 2024

added a patch:

+        if not self._action_all_configs:
+            for f in self.filtered_configurations:
+                (c, cfg) = f.split(os.sep)

re-closing...

@petersilva
Copy link
Contributor

note: shows there is a coverage issue in our tests... none of them run "sr3 start" without arguments.

@andreleblanc11
Copy link
Member

Maybe we could add disabled configs in the flow tests to increase exposure?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Sugar nice to have features... not super important.
Projects
None yet
Development

No branches or pull requests

3 participants