Skip to content

*Experimental* support for kwargs. #196

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

Closed
wants to merge 1 commit into from
Closed

*Experimental* support for kwargs. #196

wants to merge 1 commit into from

Conversation

1st1
Copy link
Member

@1st1 1st1 commented Sep 15, 2017

See issue #9 for details.

This is not ready to be committed yet. My main concern is the
prolification of 'kwargs' argument in all APIs. Perhaps a better
approach would be implemnt a Plugin/Middleware class.

See issue #9 for details.

This is not ready to be committed yet.  My main concern is the
prolification of 'kwargs' argument in all APIs.  Perhaps a better
approach would be implemnt a Plugin/Middleware class.
@1st1 1st1 requested a review from elprans September 15, 2017 20:11
@nhumrich
Copy link

+1 to plugin/middleware capabilities.

yield buf
i += 1

# Build a query that has 33000 unique and shortest possible
Copy link
Member

Choose a reason for hiding this comment

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

Let's use 32767. 33000 looks like an arbitrary limit rather than INT16_MAX.

for i, name in enumerate(count_base61()):
if i == 33000: # Max number of arguments Postgres can accept.
break
query += f'${name}+'
Copy link
Member

Choose a reason for hiding this comment

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

No f-strings on Python 3.5.

@elprans
Copy link
Member

elprans commented Sep 18, 2017

Yes, looks fairly messy with kwargs added all over the place. I also think the "middleware" approach would be somewhat cleaner.

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.

3 participants