-
-
Notifications
You must be signed in to change notification settings - Fork 758
GET entrypoint of a workflow from st2client #4791
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
Conversation
On the "sync" stuff, i know i've brought it up before. I thought about generating the client from the swagger spec. However, there is some "custom" code that gets added on the client side for some of the stuff to make the human output "pretty". Would still be nice if there was a way for the end-client CLI to get all of the new features/flags and endpoints, even if it's basic support at first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog is missing. Would be good to have one for this fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good in general. But please add unit tests. Please also update the details of this PR to provide summary and intent of the change. Why is this change needed? What is the expected outcome after the change? Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor addition required for unit tests.
|
||
endpoint = '/actions/views/entry_point/%s' % EXECUTION['id'] | ||
|
||
httpclient.HTTPClient.get.assert_called_with(endpoint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a unit tests where the status code from the API call is not OK/200.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a commit for this, but not sure if I implemented it correctly, please let me know if I should take a different approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use assertRaisesRegexp
and check that the error message is actually Not Found returned in the FakeResponse? Search for assertRaisesRegexp
in st2 and you will see plenty of examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m4dcoder sorry for the delay, I've pushed a commit to use that method. Please let me know if it's not correct.
|
||
endpoint = '/actions/views/entry_point/%s' % EXECUTION['id'] | ||
|
||
httpclient.HTTPClient.get.assert_called_with(endpoint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use assertRaisesRegexp
and check that the error message is actually Not Found returned in the FakeResponse? Search for assertRaisesRegexp
in st2 and you will see plenty of examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Need CI jobs to pass before this can be merged.
@trstruth You also need to rebase this branch. There is a merge conflict. |
@m4dcoder I've resolved the conflict, let's see if the ci builds pass this time |
This PR implements a method in the st2client module which fetches the entrypoint of a given action ref/id. This is essentially a wrapper of this endpoint.
We use the st2client package fairly extensively to facilitate tight integration between some of our internal python services and st2. This PR is one of multiple which extend st2client to wrap more of the existing st2api endpoints which are already implemented.
Random thought - is there a way we can "sync" the api and the client? ie ensure that all of the api endpoints have a corresponding client method implemented?