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

Experimental command to fetch results from the Dashboard API #60

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

padovan
Copy link
Contributor

@padovan padovan commented Dec 10, 2024

Add PoC for kci-dev results from the Dashboard API.

See the docs/results.md for details about this command. For the time being, it will be experimental as we refine how together with kernel developers how they are going to be using kci-dev.

There is a know slowdown in the API due to kernelci/dashboard#661.

Related to #57

@padovan padovan force-pushed the experimental-results-fetching branch 3 times, most recently from 3b72aec to edb6faa Compare December 10, 2024 23:42
docs/_index.md Outdated Show resolved Hide resolved
@aliceinwire
Copy link
Member

aliceinwire commented Dec 11, 2024

LGTM
I'm ok to merge this as experimental function, we can update it after.

@aliceinwire aliceinwire added this to the v0.1.1 milestone Dec 11, 2024
@robherring
Copy link

I gave it try and have a few random comments. (Not expecting all these in this PR)

I guess getting 'latest' build is dependent on the API change.

It would be nice if --commit value could be passed thru git-rev-parse so that symbolic and short commit hashes worked. That assumes you are sitting in a tree that can resolve them, so it would have to be optional. That would effectively provide a way to get the latest as well. Probably should throw an error if the commit hash isn't found. Currently it's just 0 builds, boots, tests.

I think I would make '--action' be a sub-sub-command (e.g. kci-dev results failed-build ...).

We need to be able to get successful build logs too if we want to see warnings, right? Parsed warning summaries like the dashboard are nice too.

The log names need the compiler used too or you will get name collisions.

Supporting a config file to store the giturl and branch would be nice. One way to do that is hijacking .gitconfig with your own section. This is what the b4 utility does.

@broonie
Copy link
Member

broonie commented Dec 11, 2024

Supporting a config file to store the giturl and branch would be nice. One way to do that is hijacking .gitconfig with your own section. This is what the b4 utility does.

Getting the giturl from the configured git remotes seems like a good idea in general, separately to the branch (ie, do what git does and try the supplied URL as both a literal URL and an alias configured in .git/config).

@padovan padovan force-pushed the experimental-results-fetching branch from edb6faa to f22a75c Compare December 11, 2024 23:53
@padovan
Copy link
Contributor Author

padovan commented Dec 12, 2024

Thanks for you comments @robherring and @broonie

I guess getting 'latest' build is dependent on the API change.

I believe we have that in the API yet. I'll add a task to look into that after we merge this PR

It would be nice if --commit value could be passed thru git-rev-parse so that symbolic and short commit hashes worked. That assumes you are sitting in a tree that can resolve them, so it would have to be optional. That would effectively provide a way to get the latest as well. Probably should throw an error if the commit hash isn't found. Currently it's just 0 builds, boots, tests.

Yeah. Good point. Will add a task for it as well.

I think I would make '--action' be a sub-sub-command (e.g. kci-dev results failed-build ...).

Indeed. It is my goal. But I am struggling to understand how python click does that...so I decided to focus on adding value to users like you first rather than deep diving on python machinery.

We need to be able to get successful build logs too if we want to see warnings, right? Parsed warning summaries like the dashboard are nice too.

I have to check how we handle warnings in KCIDB and Web Dashboard. This would be the lowest priority from all your comments.

The log names need the compiler used too or you will get name collisions.

Fixed in this PR.

Supporting a config file to store the giturl and branch would be nice. One way to do that is hijacking .gitconfig with your own section. This is what the b4 utility does.

Awesome idea. Thanks for this suggestion!

@padovan padovan marked this pull request as ready for review December 12, 2024 00:00
@padovan padovan force-pushed the experimental-results-fetching branch from f22a75c to 81d43d8 Compare December 12, 2024 00:44
@aliceinwire
Copy link
Member

please resolve conflicts with main.py

The results command is accessing the Maestro API directly
for fetching results. We want to build an origin agnostic
infra, so let's rename this one to 'maestro-results' for
clarity.

Signed-off-by: Gustavo Padovan <[email protected]>
Create base wrappers for normal print and error messages.

Signed-off-by: Gustavo Padovan <[email protected]>
It introduces `kci-dev results` to supporting pulling results
data in various forms from the Dashboard API.

For now it has two actions (through the --action param)

1. `--action=summary`: to get a basic numeric summary of pass/fail

2. `--action=failed-builds`: to get the list of fail builds

Addionally it has the `--download-logs` option to download the
build logs with errors.

Signed-off-by: Gustavo Padovan <[email protected]>
kci-dev is evolving beyond just the Maestro service, so we
have to re-organize the house to accomadate our new usecases.

Signed-off-by: Gustavo Padovan <[email protected]>
kci-dev is growing to meet the demand of the kernel community,
so our documenation has to follow that movement.

This commit is nothing more than just a first stab to document
better.

Signed-off-by: Gustavo Padovan <[email protected]>
Help our users understand how to use the experimental commands.

Signed-off-by: Gustavo Padovan <[email protected]>
@padovan padovan force-pushed the experimental-results-fetching branch from 81d43d8 to 597f879 Compare December 12, 2024 11:51
@padovan
Copy link
Contributor Author

padovan commented Dec 12, 2024

@aliceinwire I have this

reformatted /home/gus/p/ecosystem/kci-dev/kcidev/main.py

All done! ✨ 🍰 ✨
1 file reformatted, 12 files left unchanged.
Fixing /home/gus/p/ecosystem/kci-dev/kcidev/main.py

But no changes are generated:

(.venv) poutine kci-dev > git diff
(.venv) poutine kci-dev > git status 
On branch experimental-results-fetching
Untracked files:
  (use "git add <file>..." to include in what will be committed)
	kcidev/full-next-results.json
	kcidev/subcommands/analyze-dashboard-json.py

nothing added to commit but untracked files present (use "git add" to track)

so I don't know how to make the pipeline green :(

@aliceinwire
Copy link
Member

@aliceinwire I have this

reformatted /home/gus/p/ecosystem/kci-dev/kcidev/main.py

All done! ✨ 🍰 ✨
1 file reformatted, 12 files left unchanged.
Fixing /home/gus/p/ecosystem/kci-dev/kcidev/main.py

But no changes are generated:

(.venv) poutine kci-dev > git diff
(.venv) poutine kci-dev > git status 
On branch experimental-results-fetching
Untracked files:
  (use "git add <file>..." to include in what will be committed)
	kcidev/full-next-results.json
	kcidev/subcommands/analyze-dashboard-json.py

nothing added to commit but untracked files present (use "git add" to track)

so I don't know how to make the pipeline green :(

The following command

poetry run black .

gives me the following diff

diff --git a/kcidev/main.py b/kcidev/main.py
index a8ddeea..544b2a5 100755
--- a/kcidev/main.py
+++ b/kcidev/main.py
@@ -4,8 +4,14 @@
 import click

 from kcidev.libs.common import *
-from kcidev.subcommands import (checkout, commit, maestro_results, patch,
-                                results, testretry)
+from kcidev.subcommands import (
+    checkout,
+    commit,
+    maestro_results,
+    patch,
+    results,
+    testretry,
+)


 @click.group(

@aliceinwire
Copy link
Member

looks like isort and black are conflicting each other about the main.py import writing

@aliceinwire
Copy link
Member

sending a patch for this

@aliceinwire
Copy link
Member

This should fix it
#61
please update and rebase after is merged

@aliceinwire
Copy link
Member

aliceinwire commented Dec 12, 2024

merged, after update and rebase should output the reformatted main.py

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.

4 participants