Repository navigation
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Coverage report —
|
| Name | Stmts | Miss | Cover | Missing |
|---|---|---|---|---|
| src/kernels/__init__.py | 14 | 0 | 100% | |
| src/kernels/_system.py | 6 | 1 | 83% | 10 |
| src/kernels/_versions.py | 130 | 14 | 89% | 53, 59-60, 63-64, 102, 165-170, 199, 219 |
| src/kernels/archs.py | 56 | 1 | 98% | 94 |
| src/kernels/backends.py | 215 | 63 | 71% | 42, 46, 50-53, 70, 92, 110, 119, 123, 127-129, 150, 159, 163, 167-169, 190, 201, 203, 210-213, 222, 226, 230-255, 263, 286-306 |
| src/kernels/compat.py | 9 | 1 | 89% | 5 |
| src/kernels/deps.py | 70 | 1 | 99% | 56 |
| src/kernels/digest.py | 58 | 5 | 91% | 71-73, 136-137 |
| src/kernels/hf_hub.py | 63 | 2 | 97% | 21, 23 |
| src/kernels/importer.py | 74 | 5 | 93% | 158, 162, 165, 179-180 |
| src/kernels/install.py | 21 | 7 | 67% | 76-100 |
| src/kernels/layer/__init__.py | 7 | 0 | 100% | |
| src/kernels/layer/_interval_tree.py | 103 | 4 | 96% | 23, 52, 147, 150 |
| src/kernels/layer/device.py | 48 | 12 | 75% | 42, 47-49, 91, 96-98, 101, 149, 152, 156 |
| src/kernels/layer/func.py | 93 | 5 | 95% | 94, 119, 195, 315, 390 |
| src/kernels/layer/globals.py | 5 | 0 | 100% | |
| src/kernels/layer/inspect.py | 27 | 0 | 100% | |
| src/kernels/layer/kernelize.py | 82 | 8 | 90% | 259, 297, 305-306, 312, 316, 332-334 |
| src/kernels/layer/layer.py | 320 | 14 | 96% | 212, 259, 435, 575-576, 597, 605, 616, 657, 661, 674, 729, 770, 845 |
| src/kernels/layer/mode.py | 14 | 0 | 100% | |
| src/kernels/layer/repos.py | 144 | 36 | 75% | 33, 36-43, 74, 90, 94, 103-104, 110, 113-116, 123-124, 130, 133-136, 143-144, 150, 153-156, 163-164, 170, 173-176, 257 |
| src/kernels/load.py | 71 | 2 | 97% | 338, 378 |
| src/kernels/locking.py | 89 | 64 | 28% | 35-83, 91-98, 102-125, 137, 152-159, 165-175, 179-186 |
| src/kernels/python_deps.py | 58 | 6 | 90% | 59-60, 64-65, 101, 104 |
| src/kernels/resolver.py | 162 | 1 | 99% | 248 |
| src/kernels/status.py | 50 | 2 | 96% | 25, 79 |
| src/kernels/validate.py | 104 | 7 | 93% | 9, 101, 172, 195-196, 232-233 |
| src/kernels/variants.py | 278 | 17 | 94% | 65, 96, 117, 147, 256-257, 299-302, 304, 388-394, 400-406, 455-461 |
| src/kernels/verify.py | 102 | 6 | 94% | 44, 170-172, 260-261 |
| TOTAL | 2473 | 284 | 89% |
Updated by the Test kernels workflow on commit 3b672cfe2569063fe9e56ce2302f9ff165629092.
| If you already have an open upload pull request against a branch (for | ||
| example from an earlier upload of another build variant), the upload adds | ||
| its commits to that pull request rather than opening a new one. Pull | ||
| requests with merge conflicts are not reused. |
There was a problem hiding this comment.
I think this is very CI-specific. So, we don't likely need to include it in our docs. How other people manage it shouldn't be opioninated particularly when we don't have enough experience to substantiate.
There was a problem hiding this comment.
agreed, removed the paragraph. the flag description in the cli reference still says an open pr gets updated since that comes from the clap help, but there is no guidance on managing prs in the build docs anymore
| Possible values: `model`, `kernel` | ||
|
|
||
| * `--create-pr` — Open a pull request with the changes rather than committing them directly (does not require write access to the repository) | ||
| * `--create-pr` — Open a pull request with the changes rather than committing them directly (does not require write access to the repository). An open pull request of an earlier upload by the same user is updated instead |
There was a problem hiding this comment.
While we're at it, I think it would also make sense to clarify that we can only open a PR for a branch that exists?
There was a problem hiding this comment.
added "The target branch must already exist" to the flag description (in main.rs and both spots in builder-cli.md)
| /// Whether the pull request can be updated by an upload to `branch`. A pull | ||
| /// request with conflicts is skipped: further commits cannot resolve them. | ||
| fn is_reusable_upload_pr(details: &DiscussionDetails, branch: &str) -> bool { | ||
| details | ||
| .changes | ||
| .as_ref() | ||
| .is_some_and(|changes| changes.base == format!("refs/heads/{branch}")) | ||
| && details.files_with_conflicts.is_empty() | ||
| } |
There was a problem hiding this comment.
What is the course of action after skipping from the perspectives of our CI?
There was a problem hiding this comment.
good question, the word "skip" is a bit of a misnomer. the upload finds an open upload pr by the same user against the same branch, sees the hub already marks it as conflicting, and leaves it alone. the files from this upload go into a new pr opened from the current branch head, nothing is dropped. the old pr stays open on the hub until someone closes it
a pr only conflicts when the base branch moved under it and both sides changed the same file. esmfold hit that since torch-cuda is a universal variant, both arch jobs build it, and each job opened its own pr from the same base. merging one pair left the other pair changing _ops.py, metadata.json and the signature on a stale base, which conflicts in any merge order
with this change the second job commits into the first job's pr instead. the variant dir is overwritten in one commit (metadata and signature together), so it is one pr per branch with a linear history, same last writer wins as a direct upload. ci on its own cannot produce a conflicting pr anymore
what is left is the base moving for some other reason
- a maintainer pushes to the branch directly while an upload pr is open
- another user's upload pr for the same version is merged first
- two jobs finish within the same second and both open a pr, note in the description
none of those can be fixed by pushing more commits, the hub has no rebase for prs and a new signature never matches what landed on the base. so the upload warns
Warning: not updating pull request #3 for `v1`, it has merge conflicts in build/torch-cuda/metadata.json.sigstore
and opens the new pr. for ci that means merge the new pr and close the old one. if the old one had variants the new one does not (the same second case) rerun those jobs and they land in the new pr
hard erroring instead would drop a finished build to end up in the same place after a rerun. the old pr could also go into --output-json so the summary comment lists it, can add that here or in a follow up
f6c18fb to
3b672cf
Compare
this pr fixes
kernel-builder upload --create-propening a new hub pr on every upload, which leaves sibling prs that conflict once one of them is merged https://huggingface.co/kernels/biohub/esmfold2-trimul/discussionstldr; the kernels-community build runs
upload --create-pronce per matrix job plus again for the v1 to main step, and every call opens prs for bothmainandv1from the same base commit. a single release build after huggingface/kernels-community#1214 opened 6 prs (#2 - #7). once #6 and #7 were merged, #3 and #4 conflict onbuild/torch-cuda/metadata.json.sigstoresince each job signs the metadata separatelyupload --create-prnow looks for an open pr from the same user against the same branch (matched by the upload commit title) and pushes to itsrefs/pr/Ninstead of opening a new one. the existing files are listed from the pr so deletes are computed against what the pr already has. prs with merge conflicts are skipped since more commits cannot resolve them, and if the lookup fails it warns and falls back to opening a new prthe discussions api is not covered by
hf-hubso the lookup calls it directly with the newhub_endpointandhub_tokenhelpers inkernels-common. the--output-jsonpull requests also includeupdated: truewhen an existing pr was reusednote: two jobs that finish within the same second can still both miss each other's pr and open duplicates. moving the upload into a single job in kernels-community would remove that, but this already covers the matrix jobs, the main step and reruns
unit tests cover the title matching and the base/conflict checks
**happy to follow a different approach if this is non optimal!