Skip to content

config_context renders against incorrect pack #4570

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
Mar 13, 2019
Merged

config_context renders against incorrect pack #4570

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e2cca81
Fixes https://github.com/StackStorm/st2/issues/4567: config_context r…
jinpingh Feb 28, 2019
a97fd8e
Remove render for parent pack configuration.
jinpingh Feb 28, 2019
5c64ca0
Merge branch 'master' into issue-4567/config_context_renders
jinpingh Mar 7, 2019
62a51fb
New unit test for config_context renders against incorrect pack
jinpingh Mar 7, 2019
0e780ad
Fix review comments for unit test.
jinpingh Mar 7, 2019
0c41155
Remove code for getting action reference from spc.action
jinpingh Mar 7, 2019
f76e6dd
Integration test for config_context renders against incorrect pack
jinpingh Mar 8, 2019
82ce3c5
Remove integration test code and setup test environment from `./tools…
jinpingh Mar 9, 2019
c81f042
Change integration test pack from `dummy_pack_7` to `tests`.
jinpingh Mar 10, 2019
6f62b3c
Move tests pack from st2 repo to st2test repo.
jinpingh Mar 12, 2019
eaa3858
Add step to clone st2tests repo and copy tests pack
jinpingh Mar 12, 2019
c1f0724
Remove unused import: `mock`
jinpingh Mar 12, 2019
82fcc34
Remove authentication setup checking for simplicity
jinpingh Mar 13, 2019
3a8598e
Merge branch 'master' into issue-4567/config_context_renders
jinpingh Mar 13, 2019
c81e6d1
Change warning message.
jinpingh Mar 13, 2019
30bd1a0
Merge branch 'issue-4567/config_context_renders' of https://github.co…
jinpingh Mar 13, 2019
2cbe564
Merge branch 'master' into issue-4567/config_context_renders
jinpingh Mar 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion st2common/st2common/services/workflows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,9 @@ def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=Non
# Identify the runner for the action.
runner_type_db = action_utils.get_runnertype_by_name(action_db.runner_type['name'])

# Identify action reference
action_ref = task_ex_db.task_spec.get('spec').get('action')
Copy link
Contributor

@m4dcoder m4dcoder Feb 28, 2019

Choose a reason for hiding this comment

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

There's already an action_ref at line 519. They're not the same?

Copy link
Contributor

@m4dcoder m4dcoder Mar 7, 2019

Choose a reason for hiding this comment

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

You did not address this yet.

Copy link
Contributor Author

@jinpingh jinpingh Mar 7, 2019

Choose a reason for hiding this comment

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

It is not the same. line 519 is get block of action data from action_execution_d_b. This is get action_ref = pack_name:action_name

Not sure about You did not address this yet. means. Changed if else block to one line.

Copy link
Contributor

@m4dcoder m4dcoder Mar 7, 2019

Choose a reason for hiding this comment

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

The line numbers may have shifted. But if you go down two line from def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=None), there's already an action_ref assigned. Is it different than the action_ref begin assigned in this line.

Copy link
Contributor

@m4dcoder m4dcoder Mar 7, 2019

Choose a reason for hiding this comment

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

And if they are not the same, then you just overwrote action_ref that is saved to LiveActionDB below.

Copy link
Contributor Author

@jinpingh jinpingh Mar 7, 2019

Choose a reason for hiding this comment

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

Yes, it is different. Changed my variable name.

Copy link
Contributor

@m4dcoder m4dcoder Mar 7, 2019

Choose a reason for hiding this comment

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

I just manually tested this and they are the same value. Can you show me an example where they are different.

Copy link
Contributor Author

@jinpingh jinpingh Mar 7, 2019

Choose a reason for hiding this comment

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

My bad. I thought it is reading ActionExecution.action. Put log message and tested. Committed change.

Copy link
Contributor

@m4dcoder m4dcoder Mar 7, 2019

Choose a reason for hiding this comment

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

Ok. Good you finally see it. I thought maybe I'm not clear enough.


# Set context for the action execution.
ac_ex_ctx = {
'pack': st2_ctx.get('pack'),
Expand All @@ -545,7 +548,8 @@ def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=Non
'task_execution_id': str(task_ex_db.id),
'task_name': task_ex_db.task_name,
'task_id': task_ex_db.task_id
}
},
Copy link
Contributor

@m4dcoder m4dcoder Feb 28, 2019

Choose a reason for hiding this comment

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

Why do we need a comma here?

Copy link
Contributor Author

@jinpingh jinpingh Feb 28, 2019

Choose a reason for hiding this comment

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

Did run nosetests --nocapture st2common/tests/unit/test_param_utils.py and all passed

'ref': action_ref
}

if st2_ctx.get('api_user'):
Expand Down
12 changes: 12 additions & 0 deletions st2common/st2common/util/param.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ def render_live_params(runner_parameters, action_parameters, params, action_cont

pack = action_context.get('pack')
user = action_context.get('user')
action_ref = action_context.get('ref')

try:
config = get_config(pack, user)
Expand All @@ -299,6 +300,17 @@ def render_live_params(runner_parameters, action_parameters, params, action_cont

G = _create_graph(action_context, config)

# Retrieve config for action pack if it is different with parent
if action_ref:
action_pack = action_ref.split('.')[0]
if pack != action_pack:
try:
action_config = get_config(action_pack, user)
G = _create_graph(action_context, action_config)
except Exception as e:
LOG.info('Failed to retrieve action config for pack %s and user %s: %s' % (
action_pack, user, str(e)))

# Additional contexts are applied after all other contexts (see _create_graph), but before any
# of the dependencies have been resolved.
for name, value in six.iteritems(additional_contexts):
Expand Down