Configurable branches #312
Conversation
MagicRB
changed the title
MagicRB
force-pushed
the
configurable-branches
branch
3 times, most recently
from
b2ed161
to
6dfc495
Compare
MagicRB
force-pushed
the
configurable-branches
branch
from
6dfc495
to
4e86ba5
Compare
|
I need a second pair of eyes on 243aa6b97b4b6d062ac85c6c076676ea5acbd3ca the way I'm reading that is that updating build outputs and gcroots would execute also for PRs, which seems like a recipe for disaster. |
buildbot_nix/__init__.py
Outdated
| return branch_config.do_register_gcroot( | ||
| await s.getProperty("default_branch"), await s.getProperty("branch") | ||
| ) and await s.getProperty("event") == "push" \ | ||
| and not Path(str(gc_root)).exists() and Path(str(gc_root)).readlink() == str(out_path) |
There was a problem hiding this comment.
| and not Path(str(gc_root)).exists() and Path(str(gc_root)).readlink() == str(out_path) | |
| and not (Path(str(gc_root)).exists() and Path(str(gc_root)).readlink() == str(out_path)) |
There was a problem hiding this comment.
i suspect this throws an exception errors if the file at gc_root isn't a link. not sure where the best place to check for that is and whether we strive for that level of diligence at this stage.
There was a problem hiding this comment.
It will throw yes, but its fine ish, since we control that unless the instance admin goes out of their way to screw with it, itll be fine
There was a problem hiding this comment.
unless the instance admin goes out of their way to screw with it
hehe, poor admins take the blame :-) fine for now.
please do consider the code suggestion, it's unrelated to the exception discussion and important to fix IMO
There was a problem hiding this comment.
Ah, right precedence, will apply thanks!
buildbot_nix/__init__.py
Outdated
|
|
||
| return branch_config.do_register_gcroot( | ||
| await s.getProperty("default_branch"), await s.getProperty("branch") | ||
| ) and await s.getProperty("event") == "push" \ |
There was a problem hiding this comment.
regarding #312 (comment) i expect this to prevent registering gcroots for PRs unless the source branch of the PR is one of the branches that match the regex and has set register_gcroots = true.
There was a problem hiding this comment.
We cannot match on source branches sadly
There was a problem hiding this comment.
Without making a network requests to girhubs api, or maybe, hm ill check it again, there might be a way. Maybe it arrives over the webhook but buildbot discards it?
There was a problem hiding this comment.
i meant that if this branch that receives the push happens be a source branch for a PR then it register a GC root. under all other circumstances i foresee this to not register GC rootfs for PRs.
There was a problem hiding this comment.
The pr branch names are pull/number or smth like that
There was a problem hiding this comment.
got it, then i think your concern from the linked comment doesn't apply because the branch name won't ever match in case of a PR.
There was a problem hiding this comment.
Oh yeah youre right, my bad. I'll recheck this and make a flow chart smth, this whole process is getting very very confusing and tbh im a bit lost myself. Flow chart it is
MagicRB
force-pushed
the
configurable-branches
branch
from
243aa6b
to
6e63e22
Compare
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
…oots` for PR branches Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
MagicRB
force-pushed
the
configurable-branches
branch
from
89b16e4
to
f2041f6
Compare
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
|
Holy... maybe I should squash these commits? 16 commits is a bit excessive... |
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
Signed-off-by: magic_rb <[email protected]>
No description provided.