Skip to content

Conversation

@nmaludy
Copy link
Member

@nmaludy nmaludy commented Jul 12, 2019

Fixes #4739

Before:

$ st2 execution get 5d289e5f9387ef2ebde9513e
id: 5d289e5f9387ef2ebde9513e
status: succeeded (0s elapsed)
parameters: 
  cmd: date
result: 
  failed: false
  return_code: 0
  stderr: ''
  stdout: Fri Jul 12 10:51:11 EDT 2019
  succeeded: true

After:

$ st2 execution get 5d289e5f9387ef2ebde9513e
id: 5d289e5f9387ef2ebde9513e
action.ref: core.local
context.user: st2admin
parameters: 
  cmd: date
status: succeeded (0s elapsed)
start_timestamp: Fri, 12 Jul 2019 14:51:11 UTC
end_timestamp: Fri, 12 Jul 2019 14:51:11 UTC
result: 
  failed: false
  return_code: 0
  stderr: ''
  stdout: Fri Jul 12 10:51:11 EDT 2019
  succeeded: true

@arm4b arm4b requested review from Kami and m4dcoder July 16, 2019 12:33
Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

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

Minor question about removing liveaction.

class ActionExecutionGetCommand(ActionRunCommandMixin, ResourceViewCommand):
display_attributes = ['id', 'action.ref', 'context.user', 'parameters', 'status',
'start_timestamp', 'end_timestamp', 'result', 'liveaction']
'start_timestamp', 'end_timestamp', 'result']
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't want liveaction?

@m4dcoder
Copy link
Contributor

Also, this change resulted in errors in various unit tests which didn't expect the extra fields. This is why the travis-ci/pr job failed. Please run make tests locally to ensure tests passed.

@Kami
Copy link
Member

Kami commented Aug 3, 2019

Thanks for contribution.

It would be great to add a test case for it to avoid such regressions in the future.

@Kami Kami added this to the 3.1.1 milestone Oct 9, 2019
@arm4b arm4b removed this from the 3.1.1 milestone Oct 10, 2019
Kami added 20 commits October 29, 2019 19:23
This way we can troubleshoot various "fail to start" issues on CI.

Without this change, we have no visibility if service fails to start
before the actual service starts writting into it's own log file.

Sadly we can't rely on new version of screen which supports -Logfile
path.log flag so we need to use screen config per screen window so we
can use different log files for different processes.
…_issue

fixing subprocess to use system buffer instead of being unbuffered
…pter

Don't call eventlet directly inside the st2reactor code
…ule-parameters

/v1/rules API endpoint action parameters secrets masking (additional changes on top of StackStorm#4788)
gunicorn if auth.api_url config option was not set.

NOTE: This issue would only affect deployments using gunicorn which
don't have auth.api_url config option set.
…unicorn_issue

Fix a bug with authentication API endpoint returning internal server error under gunicorn if auth.api_url config option was not set
Copy link
Member

@Kami Kami left a comment

Choose a reason for hiding this comment

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

I verified this works as expected so I will go ahead and merge this into master.

I believe "liveaction" attribute was removed since Nick changed the code to use display_attributes defined on the class and we don't want to include "liveaction" object in st2 execution run and st2 execution get command output.

Thanks again for the contribution.

@Kami Kami added this to the 3.2.0 milestone Nov 1, 2019
@Kami Kami self-assigned this Nov 1, 2019
@Kami Kami merged commit 1a5dc9b into StackStorm:master Nov 1, 2019
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.

st2 execution get doesn't print action.ref for non-workflow actions

8 participants