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

Update action entry point API controller to return correct Content-Type response header #4327

Merged
merged 10 commits into from
Sep 11, 2018

Conversation

Kami
Copy link
Member

@Kami Kami commented Sep 3, 2018

This pull request updates GET /v1/actions/views/entry_point/<action ref> API endpoint to return correct header based on the entry point content type (yaml, python script, bash script, etc.).

Previously it would always incorrectly return application/json.

Resolves #4269.

@Kami Kami added the API label Sep 3, 2018
@Kami Kami added this to the 2.9.0 milestone Sep 3, 2018
@Kami Kami requested a review from enykeev September 3, 2018 13:43
# Special case if /etc/mime.types doesn't contain entry for yaml, py
if not content_type:
_, extension = os.path.splitext(abs_path)
if extension in ['.yaml', '.yml']:
Copy link
Member Author

Choose a reason for hiding this comment

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

I added entries for "common cases" such as YAML and Python files.

Usually default /etc/mime.types file doesn't contain entry for YAML files so the endpoint would always just return text/plain which is not desired.

@@ -531,7 +531,12 @@ def __init__(self, **entries):

try:
validator = CustomValidator(response_spec['schema'], resolver=self.spec_resolver)
validator.validate(resp.json)

response_type = response_spec['schema'].get('type', 'json')
Copy link
Member Author

Choose a reason for hiding this comment

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

@enykeev I needed to make change since our router code didn't really correctly support non JSON responses.

It always called resp.json which would fail on non-json responses.

Copy link
Contributor

@bigmstone bigmstone left a comment

Choose a reason for hiding this comment

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

LGTM, but take a look at comment.

@@ -186,10 +191,32 @@ def get_one(self, ref_or_id, requester_user):
raise StackStormDBObjectNotFoundError('Action ref_or_id=%s has no entry_point to output'
% ref_or_id)

with open(abs_path) as file:
content = file.read()
with codecs.open(abs_path, 'r') as fp:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be io.open?

@Kami Kami merged commit ffa3b6d into master Sep 11, 2018
@Kami Kami deleted the entry_point_api_endpoint_content_type_fix branch September 11, 2018 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants