Python runner actions datastore access changes, action unit testing fixes and lint cleanup #2511
Conversation
| done | ||
| # Lint Python pack management actions | ||
| . $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./lint-configs/python/.pylintrc --load-plugins=pylint_plugins.api_models contrib/packs/actions/pack_mgmt/ || exit 1; | ||
| . $(VIRTUALENV_DIR)/bin/activate; pylint -E --rcfile=./lint-configs/python/.pylintrc --load-plugins=pylint_plugins.api_models contrib/packs/actions/ || exit 1; |
There was a problem hiding this comment.
I noticed we added some new Python files there but we didn't lint files there before so some lint violations managed to slip in.
datastore management methods via the action service. This way the whole interface is consistent with the existing sensor interface (sensor_service).
…ervice to the Action class used by unit tests.
| # Methods for datastore management | ||
| ################################## | ||
|
|
||
| def list_values(self, local=True, prefix=None): |
There was a problem hiding this comment.
Why not just let datastore_service be a pass through and expect actions to access self.datastore_service directly?
There was a problem hiding this comment.
This way it's consistent with existing sensor service interface.
There was a problem hiding this comment.
Hmm, not a fan of the pattern but consistency is better in this regard.
Kami
changed the title
| self.logger = get_logger_for_action(action_name=self.__class__.__name__) | ||
|
|
||
| # action_service is late assigned post class instatiation in "setup()" | ||
| self.datastore = None |
|
Also, just a heads up. There is some similar / duplicated code in the mocks, etc. but I plan to add tests, etc. first and address / refactor that later. |
|
Likely some actions you have in place will be affected. Since this is not released yet we should be ok changing the interface. |
|
Thanks for the heads up! |
|
👍 |
|
@manasdk and I discussed the offline the other day and decided to go with another approach which is consistent with sensors. We will first try to pass This is not ideal, but good thing is that a lot of Action classes doesn't override the constructor and we control Eventually, (e.g. after next .minor) release we should also consider dropping this late assignment fall back since by then most actions should be updated. |
… to the Action class constructor. If that doesn't work (e.g. old action which hasn't been updated yet), result to late assignment.
|
I will go ahead and merge those + documentation changes into master. I updated affected actions in core packs which are bundled in this repo, but I will wait with actions in st2contrib until more users upgrade to the latest version and / or pack versioning is in place. I could update it so it also works with older versions of StackStorm, but it would require more boiler plate code... |
Python runner actions datastore access changes, action unit testing fixes and lint cleanup
This pull request updates base
Actionclass to utilizeaction_serviceargument and expose datastore management methods via thisaction_serviceinstance variable.Using
action_servicemakes it consistent with sensors and also allows us to expand and add additional functionality to action service in the future.In addition to that, it includes some other changes:
TODO