Skip to content

Api Definition by Importing OpenAPI yaml in integration-test#1569

Merged
vigoo merged 7 commits intogolemcloud:mainfrom
Nanashi-lab:import-api
May 20, 2025
Merged

Api Definition by Importing OpenAPI yaml in integration-test#1569
vigoo merged 7 commits intogolemcloud:mainfrom
Nanashi-lab:import-api

Conversation

@Nanashi-lab
Copy link
Copy Markdown
Contributor

@Nanashi-lab Nanashi-lab commented Apr 29, 2025 •
edited
Loading

closes #1559

completely fixes the issue in #1559

cause -
golem-worker-service/src/grpcapi/api_definition

            create_api_definition_request::ApiDefinition::Openapi(definition) => {
                let converted = OpenApiHttpApiDefinition(
                    serde_json::from_str(&definition).map_err(|_| bad_request("Invalid JSON"))?,
                );

fix -
Now accepts both JSON and YAML, added two more tests (works in both http-mode and grpc)
deps.done file is created while doing cargo make fix (This wasnt the case, a week+ ago)

@Nanashi-lab Nanashi-lab marked this pull request as draft April 30, 2025 07:27
@Nanashi-lab Nanashi-lab marked this pull request as ready for review April 30, 2025 09:44
@Nanashi-lab
Copy link
Copy Markdown
Contributor Author

Nanashi-lab commented Apr 30, 2025

Fixed the whole ticket

Grpc failed because it could only accept APIDefinition::OpenAPI in Json format, now it can accept both Json and Yaml

It checks for Yaml, on failure checks for Json -> and passes the value to create the api definition

There are now two more tests in integration-tests/api/api_definition.rs
create_openapi_yaml_definition and create_openapi_json_definition

Now creating api definition integration test is possible through both http and grpc mode. Please Review and Accept, thank you.

@Nanashi-lab Nanashi-lab mentioned this pull request Apr 30, 2025
Closed
11 tasks
@Nanashi-lab
Copy link
Copy Markdown
Contributor Author

Nanashi-lab commented May 7, 2025

@mschuwalow @afsalthaj Can I have review for this PR Please
Changes

In worker-service - grpc
Import now function accepts both JSON and YAML as valid imports for Creating/Updating a API

In integration-tests
Added two new tests
One to test Import using YAML - create_openapi_yaml_definition
Another using JSON - create_openapi_json_definition

@afsalthaj
Copy link
Copy Markdown
Contributor

afsalthaj commented May 19, 2025

@Nanashi-lab I am taking a look at it now

mschuwalow
mschuwalow approved these changes May 19, 2025
View reviewed changes
golem-worker-service/src/grpcapi/api_definition.rs
let converted = OpenApiHttpApiDefinition(
serde_json::from_str(&definition).map_err(|_| bad_request("Invalid JSON"))?,
);
// Try YAML first
Copy link
Copy Markdown
Contributor

@mschuwalow mschuwalow May 19, 2025

Choose a reason for hiding this comment

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

The parsing can be extracted to a function

@vigoo vigoo merged commit 440d675 into golemcloud:main May 20, 2025
22 checks passed
@Nanashi-lab Nanashi-lab deleted the import-api branch May 21, 2025 03:26
This was referenced May 22, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@afsalthaj afsalthaj afsalthaj approved these changes

@mschuwalow mschuwalow mschuwalow approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Figure why Create Api Definition by Importing OpenAPI yaml only works in http-mode

4 participants

@Nanashi-lab @afsalthaj @mschuwalow @vigoo