Skip to content
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

feat: Improve Orchestration Sample Code #141

Merged
merged 6 commits into from
Nov 7, 2024
Merged

Conversation

MatKuhr
Copy link
Member

@MatKuhr MatKuhr commented Nov 5, 2024

Context

This PR updates the orchestration sample code to be more realistic and more concise.

Also, the E2E test assertions are improved to assert on expected output.

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Aligned changes with the JavaScript SDK
  • Documentation updated
  • Release notes updated

@MatKuhr MatKuhr added the please-review Request to review a pull-request label Nov 5, 2024
.filter(entity -> entity != DPIEntities.UNKNOWN_DEFAULT_OPEN_API)
.map(entity -> DPIEntityConfig.create().type(entity))
.toList();
private static CompletionPostRequest prepareRequest(
Copy link
Member Author

Choose a reason for hiding this comment

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

helper methods are more typical and easier to understand

};
@GetMapping("/completion")
@Nonnull
public CompletionPostResponse completion() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Added a simple completion, since templating is optional (and potentially easier to do in other places of the code).

ChatMessage.create()
.role("system")
.content(
"Please evaluate the following user feedback and judge if the sentiment is positive or negative.");
Copy link
Member Author

Choose a reason for hiding this comment

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

IMO the CV was a bit too long and doesn't make the difference between anon / pseudo clear, thus the switch to this use case.

@MatKuhr MatKuhr mentioned this pull request Nov 5, 2024
8 tasks
@CharlesDuboisSAP CharlesDuboisSAP mentioned this pull request Nov 6, 2024
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP left a comment

Choose a reason for hiding this comment

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

My review is #143, it was too big

CharlesDuboisSAP and others added 2 commits November 6, 2024 16:59
* Charles review of orchestration sample code

* More fixes
@MatKuhr MatKuhr merged commit 0ce84a3 into main Nov 7, 2024
5 checks passed
@MatKuhr MatKuhr deleted the chore/sample-code-cleanup branch November 7, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please-review Request to review a pull-request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants