-
Notifications
You must be signed in to change notification settings - Fork 213
chore: onboard isort 7.0.0 precommit check #1252
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
base: master
Are you sure you want to change the base?
Conversation
|
Important Review skippedToo many files! 118 files out of 268 files are above the max files limit of 150. You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I expect this change collaborate with precommit github action check to protect the package import sequence, but somehow we removed the action by #1154. @moshemorad to provide the reason. |
|
✅ Docker image ready for
Use this tag to pull the image for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:64a89d2
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:64a89d2 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:64a89d2
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:64a89d2Patch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:64a89d2Robusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:64a89d2 |
Results of HolmesGPT evals
Legend
|
a1bbbb8 to
0c04977
Compare
|
@arikalon1 Thanks for your review. This PR involves too many files and easily have conflicts. Please help merge this PR if it's ready before we have other new changes. Thanks. |
✅ Results of HolmesGPT evalsAutomatically triggered by commit 0c04977 on branch Results of HolmesGPT evals
Time/Cost columns show % change vs historical average (↑slower/costlier, ↓faster/cheaper). Changes under 10% shown as ±0%. Historical Comparison DetailsFilter: excluding branch 'mainred/add-isort-precommit-check' Status: Success - 18 test/model combinations loaded Experiments compared (30):
Comparison indicators:
📖 Legend
🔄 Re-run evals manually
Option 1: Comment on this PR with Or with more options (one per line): Run evals on a different branch (e.g., master) for comparison:
Quick re-run: Use Option 2: Trigger via GitHub Actions UI → "Run workflow" 🏷️ Valid markers
Commands: |
It makes less code change if we get the packages import sequence aligned by isort. Isort has been used separated or integrated with IDE to sort the python import packages to make the package import clean and tidy.
I explicitly excluded sever.py, experimental/ag-ui/server-agui.py and holmes/main.py from isort check which has extra requirement for the package import sequence out of custom certification consideration.
Isort has gone through all python files in this repo, and updated the files that requires package import sequence adjustment in this PR, so please expect a large number of file change.