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

feat: cleaner python requests json snippets #189

Merged
merged 5 commits into from Dec 1, 2020

Conversation

erunion
Copy link
Contributor

@erunion erunion commented Oct 16, 2020

This cleans up the Python requests target snippets to:

  • Ensure that all dict keys are wrapped in double quotes (most everything already was).
  • For application/json requests, we now build out Python-compatible dicts and pass those to requests via the json=payload argument that it provides.
    • With this work, false, true, and null are now being translated into the Python-proper literals of False, True, and None.

@erunion erunion changed the title feat: cleaner python json snippets feat: cleaner python requests json snippets Oct 16, 2020
Copy link

@DMarby DMarby left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution!
Just one minor note about the function documentation on buildString

src/targets/python/helpers.js Show resolved Hide resolved
@erunion
Copy link
Contributor Author

erunion commented Oct 21, 2020

@DMarby Updated that docblock, and since I copied that function from src/targets/swift/helpers.js I've also updated it there since it was missing as well.

Once this library gets some Babel compilation, these string padding methods could probably removed in favor of String.prototype.padStart and String.prototype.padEnd.

@DMarby DMarby self-requested a review October 22, 2020 11:38
Copy link

@DMarby DMarby left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! I think a conflict was introduced by your other PR I just merged - #188 😅

@erunion
Copy link
Contributor Author

erunion commented Nov 27, 2020

@develohpanda I've fixed the merge conflict so this should be good to go.

@develohpanda
Copy link
Contributor

Hi @erunion, it looks like one of the tests from the merge conflict is now failing 🙁 just need to fix up the expected double-quotes in that I think!

@erunion
Copy link
Contributor Author

erunion commented Dec 1, 2020

@develohpanda Oops, sorry about that. Fixed now.

@develohpanda develohpanda merged commit e81f30a into Kong:master Dec 1, 2020
@erunion erunion deleted the feat/cleaner-python-json-usage branch December 1, 2020 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants