-
Notifications
You must be signed in to change notification settings - Fork 36
Add missing endpoints and separate generated code #324
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
base: main
Are you sure you want to change the base?
Conversation
…s and moved shared enums
This reverts commit cb9fc88.
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.
Just for posterity (and to help myself since I'm new!), I'm going to write a few notes here.
I can see this is a PR consisting overwhelmingly of mechanical refactoring, with structural improvement to support the SDK Roadmap:
The adapter pattern creates 3 files for every API (codegen + adapter + proxy).
Some insights generated by Claude Code:
✏️ Real Changes (<10% of files and LOC)
- Import path updates in ~50 example files:
- conductor.client.ai.configuration → conductor.shared.ai.enums
- conductor.client.http.models → conductor.client.adapters.models - New documentation:
- CLIENT_REGENERATION_GUIDE.md (315 lines) - Real new functionality:
- Integration API support (a handful of actual implementation files)
- Some adapter methods like TaskAdapter.to_task_result()
💡 Stats:
- ~695 files (80%) are boilerplate: Generated code + stub adapters + proxy re-exports
- ~150 files (17%) are import updates: Examples and tests updating import paths
- ~30 files (3%) are actual new code: Integration features and documentation
I'm can approve this PR on the basis that e2e tests exist (even if they're incomplete), so if these underlying structural changes don't break anything, it's probably fine. 👍🏼
(And very minor thing that need not hold up the PR: from future
imports should probably be removed going forward.)
HOWEVER I have a concern about potential overengineering that I'd like to clear up @IgorChvyrov-sm before approving this.
rethinking this after seeing #326 -- concerns about architecture and possible impacts to SDK-user (developer) experience
Changes:
Generated Missing API endpoints:
Created Resource API adapters
Created Models adapters
Introduced proxy package to achieve backward compatibility
Added integration provider functionality to OrkesIntegrationClient
Added integration tests for Orkes clients