Skip to content

fix: sync sdk with orchestration spec - #189

Merged
OscarDDD merged 40 commits into
mainfrom
sync_sdk_with_orchestration_spec
Nov 2, 2026
Merged

OscarDDD merged 40 commits into
mainfrom
sync_sdk_with_orchestration_spec

Conversation

@OscarDDD

@OscarDDD OscarDDD commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Context

Closes SAP/ai-sdk-python-backlog#58.

What this PR does and why it is needed

Definition of Done

  • Code is tested (Unit, Integration, E2E)
  • Error handling created / updated & covered by the tests above
  • Documentation updated
    • Only Public APIs are allowed to be used in documentation/tutorials/sample code
  • (Optional) Aligned changes with the JS/TS and Java SDK
  • (Optional) Release notes updated -->

@OscarDDD OscarDDD changed the title Sync sdk with orchestration spec fix:sync sdk with orchestration spec Oct 14, 2026
@OscarDDD OscarDDD changed the title fix:sync sdk with orchestration spec fix: sync sdk with orchestration spec Oct 14, 2026
@OscarDDD
OscarDDD marked this pull request as ready for review October 19, 2026 15:02
mwien
mwien previously requested changes Oct 21, 2026

@mwien mwien left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR. A few things need to be clarified (and maybe should be moved to separate tickets/PR):

  • how we support file content, especially concerning the convenience layer (see this ADR from the JS team)
  • how to support special request headers (here specifically for AI-Object-Store-Secret-Name) and if that header needs to be exposed to users at all

Another question: should we add integration tests for some of the features?

@OscarDDD

Copy link
Copy Markdown
Contributor Author

In general, some more integration tests are added and the embedding multiform functionality is removed for now since the specific cohere model is not supported and planned from orchestration side.

@OscarDDD
OscarDDD requested a review from mwien October 29, 2026 16:22

@yamaceay yamaceay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

overall lgtm, just small comments / questions from my side

@alpkom alpkom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me overall. I also see that the decisions from the meeting are applied.
And I've had a look at Marcel's review, as it was requested, and it looks like all matters have been addressed.

@OscarDDD
OscarDDD dismissed mwien’s stale review November 2, 2026 15:56

Alperen has reviewed and approved. Merging this to unblock the CI pipeline.

@OscarDDD
OscarDDD merged commit dfc851b into main Nov 2, 2026
23 of 24 checks passed
@OscarDDD
OscarDDD deleted the sync_sdk_with_orchestration_spec branch November 2, 2026 15:56
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.

4 participants