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

Refactor user-facing API models #433

Merged
merged 5 commits into from
Jan 10, 2024

Conversation

r-c-n
Copy link

@r-c-n r-c-n commented Dec 14, 2023

Related to kernelci/kernelci-core#2249

Move the definition of models used in user-facing endpoints to kernelci-core so they can be used simultaneously by code in kernelci-api and helpers and other utility functions in kernelci-core.

These changes shouldn't break anything when merged, but pending PRs might need to be rebased.

@r-c-n r-c-n added the staging-skip Don't test automatically on staging.kernelci.org label Dec 14, 2023
@r-c-n r-c-n linked an issue Dec 18, 2023 that may be closed by this pull request
@r-c-n r-c-n force-pushed the refactor-api-models-part1 branch from e7d6125 to 3b5b079 Compare December 18, 2023 08:12
@r-c-n r-c-n marked this pull request as ready for review December 18, 2023 09:24
api/auth.py Outdated Show resolved Hide resolved
@JenySadadia
Copy link
Collaborator

Could you please rebase?

@r-c-n r-c-n force-pushed the refactor-api-models-part1 branch from c967bbe to 7d49a5a Compare January 5, 2024 09:20
@r-c-n
Copy link
Author

r-c-n commented Jan 5, 2024

Could you please rebase?

Done

@r-c-n r-c-n force-pushed the refactor-api-models-part1 branch 4 times, most recently from f688968 to a2455d6 Compare January 5, 2024 15:14
@r-c-n r-c-n force-pushed the refactor-api-models-part1 branch 9 times, most recently from 8831145 to de18198 Compare January 8, 2024 12:15
@r-c-n
Copy link
Author

r-c-n commented Jan 8, 2024

@JenySadadia The tests are passing, I'll remove the test commit I added to make the Dockerfile use my own branch of kernelci-core for the docker image, to point to the upstream branch, so after that the tests will fail again. That is expected and should be fixed after kernelci/kernelci-core#2249 is merged.

@r-c-n r-c-n force-pushed the refactor-api-models-part1 branch from de18198 to dd6876c Compare January 8, 2024 12:30
@JenySadadia
Copy link
Collaborator

OK, so I have merged related kernelci-core PR. We can merge this one after updating kernelci python package.
I am working on to add pyproject.toml for it.

@r-c-n
Copy link
Author

r-c-n commented Jan 9, 2024

OK, so I have merged related kernelci-core PR. We can merge this one after updating kernelci python package. I am working on to add pyproject.toml for it.

@JenySadadia This PR doesn't depend on kernelci-core as a package now, I changed it so that it picks it from the github repo during the generation of the docker container. I know that's supposed to be deprecated soon, but that was what Guillaume and Denys suggested.

Anyway, if that's a hard requirement, let me know so I can rebase this, undo the latest changes I made and re-test again. Otherwise, this can be merged after it's tested on staging and then changed afterwards.

@r-c-n r-c-n removed the staging-skip Don't test automatically on staging.kernelci.org label Jan 9, 2024
@JenySadadia
Copy link
Collaborator

Oh, yes, it doesn't depend on the python package anymore.
Let's wait for the next staging run and we'll be good to go.

@r-c-n
Copy link
Author

r-c-n commented Jan 9, 2024

@JenySadadia Now tested on staging, I removed the 'staging-skip' tag before this morning staging started and it all seems to be working.

@JenySadadia
Copy link
Collaborator

I don't see these changes on staging https://github.com/kernelci/kernelci-api/tree/staging.kernelci.org. Maybe a merge conflict with some other PR.

@JenySadadia
Copy link
Collaborator

Do you have access to the staging VM? Otherwise, I can verify it.

Ricardo Cañuelo added 3 commits January 9, 2024 12:56
Move all the model definitions that aren't exclusively used for internal
API operations (ie. all models related to user-facing endpoints) to
kernelci-core.

Move the UserGroup model to user_models.py

Signed-off-by: Ricardo Cañuelo <[email protected]>
api/models.py now contains all the server-side pydantic model
definitions. The common server and client-facing models are in the
analogous models.py file in kernelci-core.

Signed-off-by: Ricardo Cañuelo <[email protected]>
The API code will now reference the models in kernelci-core.

Signed-off-by: Ricardo Cañuelo <[email protected]>
@r-c-n r-c-n force-pushed the refactor-api-models-part1 branch from dd6876c to 15f434b Compare January 9, 2024 11:59
@r-c-n r-c-n force-pushed the refactor-api-models-part1 branch 2 times, most recently from 16be334 to e0cc117 Compare January 9, 2024 12:22
Create a new service for running unit tests based on the api service so
that it includes the kernelci-core dependency.

Signed-off-by: Ricardo Cañuelo <[email protected]>
@r-c-n r-c-n force-pushed the refactor-api-models-part1 branch from e0cc117 to ae7b631 Compare January 9, 2024 13:47
@r-c-n
Copy link
Author

r-c-n commented Jan 9, 2024

Now we got it right!

@JenySadadia
Copy link
Collaborator

Pipeline services are not running atm. Taking a look.

@JenySadadia
Copy link
Collaborator

The issue is related to Pub/Sub. This PR seems working well.

@JenySadadia JenySadadia added this pull request to the merge queue Jan 10, 2024
Merged via the queue into kernelci:main with commit b37412d Jan 10, 2024
4 checks passed
pawiecz added a commit to pawiecz/kernelci-core that referenced this pull request Jan 16, 2024
This patch was initially submitted as kernelci/kernelci-api#380 but
required moving after merging kernelci/kernelci-api#433.

Signed-off-by: Nikolay Yurin <[email protected]>
pawiecz added a commit to pawiecz/kernelci-core that referenced this pull request Jan 16, 2024
This patch was initially submitted as kernelci/kernelci-api#380 but
required moving after merging kernelci/kernelci-api#433.

Signed-off-by: Nikolay Yurin <[email protected]>
Signed-off-by: Paweł Wieczorek <[email protected]>
github-merge-queue bot pushed a commit to kernelci/kernelci-core that referenced this pull request Jan 19, 2024
This patch was initially submitted as kernelci/kernelci-api#380 but
required moving after merging kernelci/kernelci-api#433.

Signed-off-by: Nikolay Yurin <[email protected]>
Signed-off-by: Paweł Wieczorek <[email protected]>
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.

Refactor API components into kernelci-core
3 participants