-
Notifications
You must be signed in to change notification settings - Fork 80
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
More request customization #153
Conversation
Co-authored-by: Trevor James Smith <[email protected]>
…rent python versions
Thank you @iboates for initiating this PR. it looks great in shape! |
Just FYI im on vacation until July. I might pick at this anyway in the meantime though |
No worried @iboates, take your time😀! |
…firm that custom request options get passed properly
@iamtekson @Zeitsperre I am now finished with this. You should be able to specify a dictionary of request parameters to the Originally I wanted to do it for each request, but the overwhelming DRY violations were too bothersome to ignore. I also added a few new tests while I was at it, including one specifically to test that that this works. Also, I replaced all the direct references to the requests lib with ones to the internally wrapped version. |
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.
It looks good to me. Thank you very much for this wonderful PR @iboates
Closes #133 and #82
This is a draft PR, it's not even close to being finished. It's also contingent on my tests branch PR here: #151
I have changed all the
workspace
endpoints to accommodate it. It should work, but I am wondering if it may be better to use something likerequest_params
instead ofkwargs
, just so as not to potentially interfere with other keyword arguments. Such a parameter would I guess have to be a type ofDict[String, Any]
What do you guys think? For whichever option, I can add the functionality to the rest of the endpoints.