Skip to content

Conversation

@MattDevy
Copy link
Contributor

  • removes duplicated ResponseBody
  • moves body back to Response
  • removes @variants: container from Response

Split from #5647

* removes duplicated `ResponseBody`
* moves body back to `Response`
* removes `@variants: container` from Response
@github-actions
Copy link
Contributor

Following you can find the validation changes against the target branch for the APIs.

No changes detected.

You can validate these APIs yourself by using the make validate target.

Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

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

I didn't completely follow this, but are we reverting to avoid a breaking change or what is the intention here?

@pquentin
Copy link
Member

The intention is to avoid having two classes with the same name, which breaks the Go client generator.

That said, we don't want to revert; we want to change the name of the new class.

@flobernd
Copy link
Member

That said, we don't want to revert; we want to change the name of the new class.

Ok good! That would have been my suggestion as well.

@MattDevy
Copy link
Contributor Author

@flobernd @pquentin thank you for the feedback! Let's discuss this in the spec meeting and I'll amend the PR accordingly :)

@MattDevy MattDevy marked this pull request as draft November 13, 2025 09:48
@MattDevy MattDevy marked this pull request as ready for review November 13, 2025 10:46
@MattDevy
Copy link
Contributor Author

@flobernd we reviewed this today with @pquentin and @l-trotta .
We agreed that this is the correct approach, as @variants container doesn't make sense on a Response type, since we use it mainly to allow client libs to use polymorphism on Request types.
However, responses from the server don't need this.
Since @variants container isn't needed (and make no difference) on Response types. Instead of the changes in #5622

- /** @variants container */
- export class Response {
-   body: {
-     /**
-      * Evaluation results for a classification analysis.
-      * It outputs a prediction that identifies to which of the classes each document belongs.
-      */
-     classification?: DataframeClassificationSummary
-     /**
-      * Evaluation results for an outlier detection analysis.
-      * It outputs the probability that each document is an outlier.
-      */
-     outlier_detection?: DataframeOutlierDetectionSummary
-     /**
-      * Evaluation results for a regression analysis which outputs a prediction of values.
-      */
-     regression?: DataframeRegressionSummary
-   }
+   /** @codegen_name result */
+   body: ResponseBody
+ }
+ 
+ /** @variants container */
+ export class ResponseBody {
+   /**
+    * Evaluation results for a classification analysis.
+    * It outputs a prediction that identifies to which of the classes each document belongs.
+    */
+   classification?: DataframeClassificationSummary
+   /**
+    * Evaluation results for an outlier detection analysis.
+    * It outputs the probability that each document is an outlier.
+    */
+   outlier_detection?: DataframeOutlierDetectionSummary
+   /**
+    * Evaluation results for a regression analysis which outputs a prediction of values.
+    */
+   regression?: DataframeRegressionSummary

It makes sense to simply remove the /** @variants container */

- /** @variants container */
  export class Response {

@MattDevy MattDevy merged commit 2fdf049 into main Nov 13, 2025
10 checks passed
@MattDevy MattDevy deleted the fix/revert-MlEvaluateDataFrameResponse branch November 13, 2025 11:06
@github-actions
Copy link
Contributor

The backport to 9.1 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-9.1 9.1
# Navigate to the new working tree
cd .worktrees/backport-9.1
# Create a new branch
git switch --create backport-5648-to-9.1
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2fdf04924f2c2b2ffa6822155333261919be6a0c
# Push it to GitHub
git push --set-upstream origin backport-5648-to-9.1
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-9.1

Then, create a pull request where the base branch is 9.1 and the compare/head branch is backport-5648-to-9.1.

@github-actions
Copy link
Contributor

The backport to 9.2 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-9.2 9.2
# Navigate to the new working tree
cd .worktrees/backport-9.2
# Create a new branch
git switch --create backport-5648-to-9.2
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2fdf04924f2c2b2ffa6822155333261919be6a0c
# Push it to GitHub
git push --set-upstream origin backport-5648-to-9.2
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-9.2

Then, create a pull request where the base branch is 9.2 and the compare/head branch is backport-5648-to-9.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants