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

WIP: swagger yaml defination #209

Draft
wants to merge 1 commit into
base: python-3
Choose a base branch
from

Conversation

p3ck
Copy link
Contributor

@p3ck p3ck commented Jan 31, 2024

Using openapi 3.0 specification to define the restAPI for beaker backend.

skeleton api with some dummy methods to allow for testing the test framework.

Once this is fleshed out we can start migrating the model code over and then create the actual rest methods.

Created a test_systems unittest to make sure I understood how this would all glue together.
pytest output:

================================================= test session starts =================================================
platform linux -- Python 3.12.1, pytest-7.4.4, pluggy-1.3.0
rootdir: /home/bpeck/Sandbox/beaker/beaker/API
plugins: anyio-4.2.0
collected 4 items                                                                                                     

tests/test_api.py .                                                                                             [ 25%]
tests/test_systems.py ...                                                                                       [100%]

================================================== warnings summary ===================================================
============================================ 4 passed, 7 warnings in 0.10s ============================================

Alembic migrations will be handled by command line db-init and db-migrate tools.

@p3ck p3ck self-assigned this Jan 31, 2024
@p3ck p3ck marked this pull request as draft January 31, 2024 21:33
@JohnVillalovos
Copy link
Collaborator

@p3ck This looks cool. Thanks!

Only thought I have is that at some point it would be good to add some CI to test this.

@p3ck
Copy link
Contributor Author

p3ck commented Feb 1, 2024

@p3ck This looks cool. Thanks!

Only thought I have is that at some point it would be good to add some CI to test this.

Absolutely! You notice that I have the beginnings of tests in this PR. We will enable them in github when we switch over.

Lot more work to be done before then though.

Using openapi 3.0 specification to define the restAPI for beaker
backend.

skeleton api with some dummy methods to allow for testing the test
framework.

Once this is fleshed out we can start migrating the model code over and
then create the actual rest methods.
Copy link
Member

@mdujava mdujava left a comment

Choose a reason for hiding this comment

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

I took a look at a spec in a Swagger Editor and it reported few errors, commented on the affected lines.

description: "Successfully deleted system problem report entry"
'400':
$ref: '#/components/responses/400Error'
/systems/{fqdn}/problem-reports/{id}:
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate key

description: "Successfully returned Job set's details"
'400':
$ref: '#/components/responses/400Error'
/jobs/{jID}/sets/{sID}/recipes/{rID}:
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate key

put:
tags:
- Systems
description: "Update a System"
Copy link
Member

Choose a reason for hiding this comment

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

I was not sure at first if PUT is the right choice.
PUT is defined as request method creates a new resource or replaces a representation of the target resource as used in this endpoint.
Maybe adding PATCH would make it complete. (e.g. for simple change of only state)

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.

3 participants