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

frontend: first steps towards removing unnecessary APIv3 abstractions #3228

Closed
wants to merge 1 commit into from

Conversation

FrostyX
Copy link
Member

@FrostyX FrostyX commented Apr 20, 2024

We intentionally left a huge mess and a lot of magic in fields.py, schemas.py, and docs.py because it made the migration to Flask-RESTX a lot easier and we didn't know the correct abstractions beforehand.

In this patch, I am proposing several refactors to simplify the code:

  • Schemas duplicated file types that were already-defined in fields.py. I am using the already defined ones to avoid duplication
  • There seems to be no need for separately defining schemas and models now. In the end, we care only about models and schemas were IIRC only an implementation detail. I am showing how to remove some schemas and define models directly.
  • Having every field defined as a variable in one giant pile in fields.py was a temporary measure. We can define the fields within models and then point to them to avoid duplication. See project_fork_input_model and its ownername.

We intentionally left a huge mess and a lot of magic in `fields.py`,
`schemas.py`, and `docs.py` because it made the migration to
Flask-RESTX a lot easier and we didn't know the correct abstractions
beforehand.

In this patch, I am proposing several refactors to simplify the code:

- Schemas duplicated file types that were already-defined in
  `fields.py`. I am using the already defined ones to avoid duplication
- There seems to be no need for separately defining schemas and models
  now. In the end, we care only about models and schemas were IIRC
  only an implementation detail. I am showing how to remove some
  schemas and define models directly.
- Having every field defined as a variable in one giant pile in
  `fields.py` was a temporary measure. We can define the fields within
  models and then point to them to avoid duplication. See
  `project_fork_input_model` and its `ownername`.

Also, some fields (projectname, persistent, and additional_repos) were
missing from the schema while being mentioned in
`apiv3_projects.to_dict`. I don't know how it worked without errors. I
am adding them.
@FrostyX
Copy link
Member Author

FrostyX commented Apr 20, 2024

Do you think this approach could be done for the whole APIv3 code, or we will hit a wall somewhere?
The information at the swagger page didn't seem to break and beaker tests mostly pass with exception to:

delete-build 2918198 2918213 2918219

which tracebacks. But I don't think I caused it in this PR (not sure).

@praiskup
Copy link
Member

We'll have to wait for the Pydantic support in python3-flask-restx.

The thing Jakub propose works on "outputs", but not on "user inputs" int he API.

@praiskup praiskup closed this Apr 29, 2024
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