feat(client): first pass at .run() helper#5
Conversation
|
@zeke It's the same version that has been discussed in the staging repo. Do you want to already merge it as-is? |
|
what was the prompt!!? haha |
|
Added context from staging repo: https://github.com/stainless-sdks/replicate-client-python/pull/5 |
zeke
left a comment
zeke
left a comment
There was a problem hiding this comment.
Thanks for opening this, @dgellow
A few thoughts:
- Can you add a description to the PR body with a summary of the changes? What's covered? What's missing?
- There seem to be a lot of changes to the auto-generated code here. Are there any mitigations we can make here to minize that? Moving code to lib, adding code comments around human code, moving human code to a separate block in the file, etc.?
- Is it possible to add some tests?
I think that we already extracted to
Edit: Paraphrasing what I said in Slack: I'm really not too sure about adding region comments to delimitate human code vs generated code. The main issue I see is that there is no guarantee they will stay correct over time, it is possible that a git operation ends up with generated code between those comments. So that would be something that you have to maintain yourself. What I would instead recommend is to document branch comparison in CONTRIBUTING.md, such as https://github.com/replicate/replicate-typescript-stainless/compare/generated..next. |
|
Still a bit of work to do on tests — I'm using https://github.com/replicate/replicate-python/blob/main/tests/test_run.py for the list of test cases. |
|
Awesome! Excited to see the progress. |
|
I've created an issue to track this work: I know this PR has been tossed around a bit as folks move on and off of projects, or are on leave. @mpatankar who is the current owner of this? |
|
@RobertCraigie is the assignee and the one who will see this through! Assigned him to the issue you created |
| "anyio>=3.5.0, <5", | ||
| "distro>=1.7.0, <2", | ||
| "sniffio", | ||
| "asyncio>=3.4.3", |
There was a problem hiding this comment.
this isn't quite right, asyncio is stdlib
|
Thanks for the update, @dtmeadows. Great to see this getting close! |
|
@zeke this is ready for your review! The only remaining pending item is the file upload functionality that we discussed here: https://stainless-workspace.slack.com/archives/C08H2L2E0KT/p1745613017872449 Would your preference be for us to wait and have that be code-generated (once it's added to your OpenAPI spec) or to just manually create the endpoint for now? |
|
Amazing! Great timing. I actually just shipped a change to our OpenAPI schema to document the Files API, so we can generate them. That should be live in production within the next few minutes.
|
|
|
||
| from replicate import AsyncReplicate | ||
|
|
||
| client = AsyncReplicate() |
There was a problem hiding this comment.
Can we rename client to replicate here? Will that work?
There was a problem hiding this comment.
done!
(we can't change the from replicate import AsyncReplicate unfortunately though here, since the module client only returns the sync client)
zeke
left a comment
zeke
left a comment
There was a problem hiding this comment.
This is looking great. Left a few small suggestions but nothing major.
Happy to ship this without File upload support. It would probably be easier to review that in a separate PR anyway.
Sounds good! I've added file upload support on this branch: |
|
@zeke I think this is good to merge in now! You okay blessing it with your approval? |
5 participants
Part of #11