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

Save OpenAPI definition as json, generate client and run tests #969

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

ChuckMoe
Copy link
Contributor

See #958

@ChuckMoe
Copy link
Contributor Author

To further work on this PR, I have some questions:

  1. Which languages do you want to generate code for now? For all of them, please create repositories and add the credentials as environment variables in the project settings.
  2. At the moment I am using schemathesis to run automatically generated tests. This is not the best solution, but it would us get started immediately. In theory. It seems like the backend and the definition are still not aligned. To try it our yourself, execute schemathesis locally.
  3. If I start the node server in one part of a step and want to call its API in another part, is it still running or simply exited?
  4. Should I create some rudimentary tests for one client in order to test it now?

Run Schemathesis

Install

pip3 install --user schemathesis

Run

Prepare the group variables in your env in a way, that your user is allowed everything.

st run \
  -H "Authorization: Bearer TOKEN" \
  --checks all \
  --validate-schema true \
  http://localhost:3000/explorer-json

Copy link
Contributor

@bpedersen2 bpedersen2 left a comment

Choose a reason for hiding this comment

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

Some minor changes, but overall this looks good.

.github/workflows/publish_openapi_client.yaml Show resolved Hide resolved
@ChuckMoe ChuckMoe marked this pull request as ready for review January 23, 2024 07:54
@ChuckMoe
Copy link
Contributor Author

The list of generated clients can also be moved into an environment variable of the repository. That way, there is no need to change the code in the case you want to add or remove one.

.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
@ChuckMoe ChuckMoe force-pushed the openapi-workflows branch 8 times, most recently from bdb8cb9 to ae9fce7 Compare January 23, 2024 09:44
.github/workflows/test.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@dylanmcreynolds dylanmcreynolds left a comment

Choose a reason for hiding this comment

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

This looks great to me. I had one quick comment on the new mongo docker-compose. While reading, I hadn't realized that that was probably there for the github action and not local development, so maybe my comment does not really apply.

docker-compose.mongo.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@dylanmcreynolds dylanmcreynolds left a comment

Choose a reason for hiding this comment

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

This looks great to me. I had one quick comment on the new mongo docker-compose. While reading, I hadn't realized that that was probably there for the github action and not local development, so maybe my comment does not really apply.

@ChuckMoe
Copy link
Contributor Author

ChuckMoe commented Feb 1, 2024

I tried writing the tests for the generated javascript-client and typescript-client but both had some problems. Is it alright, if I write the tests for the python client?

@ChuckMoe
Copy link
Contributor Author

ChuckMoe commented Feb 7, 2024

One big open question for me is still where to save/upload the client code to. Is it supposed to be a mono-repo or should they be uploaded into different repositories?

@ChuckMoe ChuckMoe marked this pull request as draft February 7, 2024 15:56
@bpedersen2
Copy link
Contributor

I guess the mongoose-managed fields ( createdAt )etc. should have a default in the schema definition for the HistoryClass to match the other schemas.

@bpedersen2
Copy link
Contributor

#1132 should fix the HistoryClass errors

@nitrosx
Copy link
Contributor

nitrosx commented Mar 25, 2024

Regarding the test failing: are we able to download the openapi.json file and run the api creation successfully locally?
Do we have instructions on how to do that?

@ChuckMoe
Copy link
Contributor Author

@nitrosx You can download or create it yourself but it won't change the outcome. The same error will appear until the example value of updatedAt in issue #1055 is fixed. You could use --skip_validation but that would also skip every other potential error in the schema, so I advocate against it.
If you want to create the schema locally take a look at this tutorial on how to run OpenAPI on your machine.

@nitrosx
Copy link
Contributor

nitrosx commented Mar 25, 2024

@ChuckMoe thank you for clarifying.
I will look into it and make sure that somebody from the community will work on it.

@sbliven
Copy link
Contributor

sbliven commented Sep 10, 2024

Is this PR superseded by the Core Release SDK autogeneration and pySciCat project? The plan is to generate the SDK in a different repo, right? Should this PR be closed?

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.

5 participants