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

Rework kci node commands #2204

Merged
merged 2 commits into from
Nov 29, 2023
Merged

Conversation

JenySadadia
Copy link
Collaborator

Rework commands to interact with Node objects using Click framework.

@JenySadadia JenySadadia changed the title kernelci.cli.node: rework kci node commands Rework kci node commands Nov 21, 2023
Copy link
Contributor

@gctucker gctucker left a comment

Choose a reason for hiding this comment

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

Oh, I already did this work and was waiting for the other PRs to be reviewed and merged first. Well that's OK I can rebase my changes on top of this one. I'm reworking the API bindings with a Node class as part of it.

kci Outdated Show resolved Hide resolved
@JenySadadia
Copy link
Collaborator Author

Oh, I already did this work and was waiting for the other PRs to be reviewed and merged first. Well that's OK I can rebase my changes on top of this one. I'm reworking the API bindings with a Node class as part of it.

Okay. As per yesterday's meeting, @nuclearcat is blocked as kci commands are not yet ready. That's why I started reworking node commands.

@gctucker
Copy link
Contributor

@nuclearcat Why are you blocked?

@nuclearcat
Copy link
Member

@gctucker i was blocked cause i need to have stable snapshot of kernelci-core with fully functional tools.

Tested:

./kci node find --indent 2 revision.commit="041e10ebbdab4922c4f16cbcb68f6cfb82a03dfd"
./kci node count revision.commit="041e10ebbdab4922c4f16cbcb68f6cfb82a03dfd"
./kci node get 655f810d66f556944feedbb4

But for submitting new node:

 ./kci-staging.sh node submit
{"id": "655f810d66f556944fee0000", "kind": "node", "name": "checkout", "path": ["checkout"], "group": null, "revision": {"tree": "kernelci", "url": "https://github.com/kernelci/linux.git", "branch": "staging-mainline", "commit": "041e10ebbdab4922c4f16cbcb68f6cfb82a03dfd", "describe": "staging-mainline-20231123.0", "version": {"version": 6, "patchlevel": 7, "sublevel": null, "extra": "-rc2-30-g041e10ebbdab", "name": null}}, "parent": null, "state": "done", "result": null, "artifacts": {"tarball": "https://kciapistagingstorage1.file.core.windows.net/staging/linux-kernelci-staging-mainline-staging-mainline-20231123.0.tar.gz?sv=2022-11-02&ss=f&srt=sco&sp=r&se=2024-10-17T19:19:12Z&st=2023-10-17T11:19:12Z&spr=https&sig=sLmFlvZHXRrZsSGubsDUIvTiv%2BtzgDq6vALfkrtWnv8%3D"}, "data": null, "created": "2023-11-23T16:42:53.185000", "updated": "2023-11-23T17:43:00.901000", "timeout": "2023-11-23T17:42:53.097000", "holdoff": "2023-11-23T17:03:36.461000", "owner": "nuclearcat", "user_groups": []}

We have small issue :

requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://staging.kernelci.org:9000/latest/node/655f810d66f556944fee0000

I believe because submit uses PUT /node/xxx which is supposed to update node, but not create, so maybe instead of submit we can split this command to two:

  • new (or keep name submit?)
  • update
    Where new will use POST, and update will use PUT

@gctucker
Copy link
Contributor

@nuclearcat OK then we should take a snapshot of the components earlier in the git history. That's as per requested on the GitHub issue you created. The only problem is that we can have a set of revisions working together but not with the command line tool. Do you need the command line tool working in the first release candidate to prepare the production deployment?

@nuclearcat
Copy link
Member

Yes, definitely i need command line tool working and matching with new features, and i believe it is entirely possible.

@JenySadadia
Copy link
Collaborator Author

@gctucker i was blocked cause i need to have stable snapshot of kernelci-core with fully functional tools.

Tested:

./kci node find --indent 2 revision.commit="041e10ebbdab4922c4f16cbcb68f6cfb82a03dfd"
./kci node count revision.commit="041e10ebbdab4922c4f16cbcb68f6cfb82a03dfd"
./kci node get 655f810d66f556944feedbb4

But for submitting new node:

 ./kci-staging.sh node submit
{"id": "655f810d66f556944fee0000", "kind": "node", "name": "checkout", "path": ["checkout"], "group": null, "revision": {"tree": "kernelci", "url": "https://github.com/kernelci/linux.git", "branch": "staging-mainline", "commit": "041e10ebbdab4922c4f16cbcb68f6cfb82a03dfd", "describe": "staging-mainline-20231123.0", "version": {"version": 6, "patchlevel": 7, "sublevel": null, "extra": "-rc2-30-g041e10ebbdab", "name": null}}, "parent": null, "state": "done", "result": null, "artifacts": {"tarball": "https://kciapistagingstorage1.file.core.windows.net/staging/linux-kernelci-staging-mainline-staging-mainline-20231123.0.tar.gz?sv=2022-11-02&ss=f&srt=sco&sp=r&se=2024-10-17T19:19:12Z&st=2023-10-17T11:19:12Z&spr=https&sig=sLmFlvZHXRrZsSGubsDUIvTiv%2BtzgDq6vALfkrtWnv8%3D"}, "data": null, "created": "2023-11-23T16:42:53.185000", "updated": "2023-11-23T17:43:00.901000", "timeout": "2023-11-23T17:42:53.097000", "holdoff": "2023-11-23T17:03:36.461000", "owner": "nuclearcat", "user_groups": []}

We have small issue :

requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://staging.kernelci.org:9000/latest/node/655f810d66f556944fee0000

I believe because submit uses PUT /node/xxx which is supposed to update node, but not create, so maybe instead of submit we can split this command to two:

  • new (or keep name submit?)
  • update
    Where new will use POST, and update will use PUT

@nuclearcat As per the current implementation, kci node submit can be used to create/update nodes.
If you send id field in the request dictionary, It will go and update the node matching ID. Otherwise, it will create a new node.
Yes, we can also split the command into two separate commands maybe add or create and update commands if you think it will be better from the user's pov.

Jeny Sadadia added 2 commits November 28, 2023 17:22
Rework commands to interact with Node objects
using `Click` framework.

Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>
As the `kci node` commands have been reworked
and added to `kernelci.cli` module, drop `node.py`
from legacy module.

Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>
@JenySadadia
Copy link
Collaborator Author

Rebased.

@nuclearcat
Copy link
Member

Retested, all looks ok, thanks!

@nuclearcat nuclearcat added this pull request to the merge queue Nov 29, 2023
Merged via the queue into kernelci:main with commit 0ca2106 Nov 29, 2023
4 checks passed
@JenySadadia JenySadadia deleted the rework-kci-node branch November 29, 2023 06:32
@gctucker
Copy link
Contributor

gctucker commented Dec 1, 2023

OK then we're changing the plans for the first release candidate, it will be using the new user management model and the reworked kci tool rather than the previous "stable" set of components. That's fine, but the reason why I was suggesting to pick earlier versions was to decouple the production deployment from any on-going development changes and also to test migrations when deploying the new user management scheme.

@gctucker gctucker mentioned this pull request Dec 15, 2023
40 tasks
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