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

[OpenAPI] Trim operation name prefixes #20

Merged
merged 14 commits into from
Sep 30, 2024
Merged

Conversation

newtork
Copy link
Contributor

@newtork newtork commented Aug 14, 2024

Context

AI/ai-sdk-java-backlog#53.

I've noticed the internal OpenAPI generator has means to remove operation name prefixes. This way the method names in API classes can be defused.

<additionalProperties>
  <removeOperationIdPrefix>true</removeOperationIdPrefix>
  <removeOperationIdPrefixDelimiter>\.</removeOperationIdPrefixDelimiter>
  <removeOperationIdPrefixCount>3</removeOperationIdPrefixCount>
</additionalProperties>

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

newtork added 11 commits August 8, 2024 15:26
# Conflicts:
#	core/src/main/java/com/sap/ai/sdk/core/client/ApplicationApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ArtifactApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ConfigurationApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/DeploymentApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/DockerRegistrySecretApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ExecutableApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ExecutionApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ExecutionScheduleApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/FileApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/KpiApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/MetaApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/MetricsApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ModelApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ObjectStoreSecretApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/RepositoryApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ResourceApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ResourceGroupApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ResourceQuotaApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ScenarioApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/SecretApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ServiceApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/model/BckndDeploymentQuotaItem.java
#	core/src/main/java/com/sap/ai/sdk/core/client/model/BckndDeploymentResourceQuotaResponse.java
#	core/src/main/java/com/sap/ai/sdk/core/client/model/BckndDeploymentUsage.java
#	core/src/main/java/com/sap/ai/sdk/core/client/model/BckndUsageResourcePlanItem.java
#	e2e-test-app/src/main/java/com/sap/ai/sdk/app/controllers/ModelsController.java
…-prefix

# Conflicts:
#	core/src/main/java/com/sap/ai/sdk/core/Core.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ApplicationApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ArtifactApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ConfigurationApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/DeploymentApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/DockerRegistrySecretApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ExecutableApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ExecutionApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ExecutionScheduleApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/FileApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/KpiApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/MetaApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/MetricsApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ObjectStoreSecretApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/RepositoryApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ResourceApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ResourceGroupApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ResourceQuotaApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ScenarioApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/SecretApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ServiceApi.java
#	core/src/test/java/com/sap/ai/sdk/core/client/ArtifactUnitTest.java
#	core/src/test/java/com/sap/ai/sdk/core/client/ConfigurationUnitTest.java
#	core/src/test/java/com/sap/ai/sdk/core/client/DeploymentUnitTest.java
#	core/src/test/java/com/sap/ai/sdk/core/client/ExecutionUnitTest.java
#	core/src/test/java/com/sap/ai/sdk/core/client/ScenarioUnitTest.java
@newtork newtork marked this pull request as draft August 14, 2024 07:54
Comment on lines 36 to 38
public AiModelList getModels() {
return API.modelsGet("foundation-models", "default");
return API.get_0("foundation-models", "default");
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Major)

This is a problem.

Cause: Following operations resolve to the same method names + signature (String, String):

get:
  tags:
  - Scenario
  summary: Get information about all models available in LLM global scenario
  description: Retrieve information about all models available in LLM global scenario
  operationId: models.get
get:
  tags:
  - Scenario
  summary: Get scenario by id
  description: Retrieve details for a scenario specified by scenarioId.
  operationId: scenario.get

If it was fixed, we could use the logic just fine:

- operationId: models.get
+ operationId: scenario.get_models

I created a PR to address this inconsistency here

Copy link
Contributor

Choose a reason for hiding this comment

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

The fix has been merged

# Conflicts:
#	core/pom.xml
#	core/src/main/java/com/sap/ai/sdk/core/Core.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ApplicationApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ArtifactApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ConfigurationApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/DeploymentApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/DockerRegistrySecretApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ExecutableApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ExecutionApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ExecutionScheduleApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/FileApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/KpiApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/MetaApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/MetricsApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ObjectStoreSecretApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/RepositoryApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ResourceApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ResourceGroupApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ResourceQuotaApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ScenarioApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/SecretApi.java
#	core/src/main/java/com/sap/ai/sdk/core/client/ServiceApi.java
#	core/src/test/java/com/sap/ai/sdk/core/client/ArtifactUnitTest.java
#	core/src/test/java/com/sap/ai/sdk/core/client/ConfigurationUnitTest.java
#	core/src/test/java/com/sap/ai/sdk/core/client/DeploymentUnitTest.java
#	core/src/test/java/com/sap/ai/sdk/core/client/ExecutionUnitTest.java
#	core/src/test/java/com/sap/ai/sdk/core/client/ScenarioUnitTest.java
#	sample-code/spring-app/src/main/java/com/sap/ai/sdk/app/controllers/DeploymentController.java
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.

@newtork please review my commits before merging

@CharlesDuboisSAP CharlesDuboisSAP marked this pull request as ready for review September 30, 2024 08:51
Copy link
Contributor Author

@newtork newtork left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for maintenance @CharlesDuboisSAP

@newtork newtork merged commit 7a825f2 into main Sep 30, 2024
5 checks passed
@newtork newtork deleted the openapi/method-naming-prefix branch September 30, 2024 14:54
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.

2 participants