Commit Graph

169 Commits

Author SHA1 Message Date
Xavier Morel
036ae3a8ee [IMP] forwardbot: reduce length of fw branch name
* shorten the postfix, forwardbot is now a bigram!
* shorten the uniquifier: go from 5 to 3 bytes, and use urlsafe base64
  that way we only have a 4-char uniquifier instead of 8
* while at it, fix deprecated calls to logging.warn (should be
  logging.warning)

Fixes #226
2019-10-10 11:37:27 +02:00
Xavier Morel
d453943252 [IMP] *: unify gh test API between runbot and fw-bot
The fw-bot testing API should improve the perfs of mergebot tests
somewhat (less waiting around for instance).

The code has been updated to the bare minimum (context-managing repos,
change to PRs and replacing rolenames by explicit token provisions)
but extra facilities were used to avoid changing *everything*
e.g. make_commit (singular), automatic generation of PR refs, ...

The tests should eventually be updated to remove these.

Also remove the local fake / mock. Being so much faster is a huge
draw, but I don't really want to spend more time updating it,
especially when fwbot doesn't get to take advantage. A local /
lightweight fake github (as an external service over http) might
eventually be a good idea though, and more applicable (including to
third-parties).
2019-10-10 10:11:48 +02:00
Xavier Morel
fafa7ef437 [IMP] *: attempt to avoid some of the FP spam
Attempt to avoid some of the comment spam by dedup-ing input (only
signaling when the status actually changes and ignoring identity
transformations) and in case of failing CI keeping the last failed
status and not signaling on the next update if it's the same failure.

Closes #225
2019-10-07 16:38:14 +02:00
Xavier Morel
85035ad2c0 [IMP] runbot_merge: separate splits from other awaiting PRs
Should make the (eventual) wait and the extent of the splitting clearer.

Fixes #217
2019-10-07 09:26:27 +02:00
Xavier Morel
60c8f0f498 [FIX] runbot_merge: behaviour when no CI are required
If the required_statuses are empty, PRs should always be
validated (and just require a review) rather than never be merge-able.

Fixes #216
2019-10-04 10:00:14 +02:00
Xavier Morel
0152271fb8 [IMP] runbot_merge: layout responsivity
* only show 2 stagings on cellphones as 4 is way too much, moving to a
  vertical layout would probably be a bad idea as stagings can already
  be very tall and then we have multiple branches stacked on one
  another, unless we also make branches foldable

  the more complete list of stagings (per branch) is available on the
  branch's page anyway so providing a not-completely-broken home looks
  more useful, and at a fundamental level the current / last staging
  is really the one we care about
* remove the size bounds on stagings to avoid smushing all the cells
  together and overlapping text, sadly can't overflow scroll the
  stagings element because you can't have an overflow-x: scroll and an
  overflow-y: visible (that becomes auto)
2019-10-03 11:19:00 +02:00
Xavier Morel
0ae01c6ddb [FIX] runbot_merge: dropdown in branch staging history
Had apparently been fixed "live" but not committed in code, so every
update of the module would re-break it.

Fixes #221
2019-10-02 17:49:54 +02:00
Xavier Morel
e49b112447 [FIX] runbot_merge: only update pending staging state
The staging validation routine would ignore stagings which were
cancelled or ff_failed, but it should also have ignored failed and
successful aka all terminal state.

Simplify the condition for that: just ignore a staging's validation if
the staging is not pending.

Closes #211
2019-10-02 17:49:54 +02:00
Xavier Morel
a5794a1a24 [FIX] forwardport: better version of previous fix
Turns out we don't want to close the cursor on success, we just want to
commit, but that's not what the default context manager does.

So don't use said context manager.
2019-10-01 09:57:35 +02:00
Xavier Morel
eb9eeb670a [IMP] forwardport: avoid locking cron when a _validate blows up
If a _validate call blows up, the entire Commit._notify cron gets
stuck, which is an issue because not only does it stop creating
forward ports, it also stops "progressing" stagings.
2019-10-01 07:56:24 +02:00
Xavier Morel
7659293a2b [IMP] runbot_merge: update staging timeout on 'pending' CI
If the CI is greatly backed up (either insufficient capacity or jobs
spike) a timeout which is normally perfectly fine might be
insufficient e.g. given a 2h timeout, if a job normally takes 80mn but
the staging's job starts 40mn after the staging was actually created
we're sunk. And cancelling the staging once the job has finally gotten
started is not going to improve load on the CI, it just wastes a CI
slot.

Therefore assume a `pending` event denotes the actual start of the job
on the CI, and reset the timeout to start from that moment so
ci_timeout is the timeout of the CI job itself, not of the staging
having been created.

Closes #202
2019-09-23 15:42:18 +02:00
Xavier Morel
78ad4b4e4b [IMP] runbot_merge, forwardport: consolidate conftests
Converge the pytest setups of runbot_merge and forwardport a bit
more (the goal is obviously to eventually share the infrastructure so
they run the same way).
2019-09-23 13:54:42 +02:00
Xavier Morel
66d65ba550 [IMP] runbot_merge, forwardport: variable-user feedback
Having all the feedback be sent by the mergebot user (github_token) is
confusing. Add a way to specify which field of project should be used to
source the token used when sending feedback.

Fixes #190
2019-09-21 15:23:42 +02:00
Xavier Morel
52699d901a [IMP] forwardport: ping on CI failure
It's especially important as users / assignees don't get
pinged *during* the forwardport process.

closes #203
2019-09-18 08:32:38 +02:00
Xavier Morel
f671dcc828 [ADD] forwardbot
* Cherrypicking is handrolled because there seems to be no easy way to
  programmatically edit commit messages during the cherrypicking
  sequence: `-n` basically squashes all commits and `-e` invokes a
  subprocess. `-e` with `VISUAL=false` kinda sorta works (in that it
  interrupts the process before each commit), however there doesn't
  seem to be clean status codes so it's difficult to know if the
  cherrypick failed or if it's just waiting for a commit of this step.

  Instead, cherrypick commits individually then edit / rewrite their
  commit messages:

  * add a reference to the original commit
  * convert signed-off-by to something else as the original commit was
    signed off but not necessarily this one

* Can't assign users when creating PRs: only repository collaborators
  or people who commented on the issue / PR (which we're in the
  process of creating) can be assigned.

  PR authors are as likely to be collaborators as not, and we can have
  non-collaborator reviewers. So pinging via a regular comment seems
  less fraught as a way to notify users.
2019-09-05 10:00:07 +02:00
Xavier Morel
1b1aa637fe [IMP] runbot_merge: commit message edition abstraction
Prepares for more complex edition operations on the forwardbot side

* split out the pseudo-headers from the message body
* don't separate the co-authored-by headers from the others, seems
  unnecessary, we just need to ensure they're at the end so github
  doesn't miss them (/it)
2019-09-05 10:00:07 +02:00
Xavier Morel
ef24adad88 [FIX] runbot_merge: cancel button on staging
* split action_cancel (UI button) from cancel (internal): since the
  xhr mapping is weird, if there are available args the mapper thinks
  it should pass the call context as reason which is unexpected
* make cancel a no-op when called on already inactive stagings
* make cancel work when called on multiple statgings
* make computing the active staging work properly in an
  active_test=False context (e.g. when it's interacted with from the
  form view because that comes from the list view which is
  active_test=False, probably so we can see not just the stagings but
  recursively see deactivated batches in deactivated stagings)
* don't show the cancel button on inactive stagings
2019-08-27 12:28:53 +02:00
Xavier Morel
b1b959d472 [FIX] runbot_merge: properly handle retarget from deactivated branch
A deactivated branch is generally treated as unmanaged which is mostly
correct except for the case of retargeting an existing PR.

When a branch is deactivated the corresponding PRs are not removed, so
it's possible to have live PRs associated with ~unmanamaged
branches. When retargeting those PRs to active branches, the mergebot
would assume there was no existing PR and would create a duplicate,
then either get completely lost (before
a84595ea04) or blow up (after the same).

Properly search amongst deactivated branches for retargeting sources
so we update the relevant PR instead of trying to create duplicates.

Fixes #169
2019-08-27 11:27:02 +02:00
Xavier Morel
e40e814b90 [IMP] runbot_merge: show heads on stagings
Stagings have a "statuses" field which was shown but useless (as it's
a binary), they also have a "heads" field which only provides a
mapping of repository names to commits.

This change provides the staging heads as a commits m2m.

Fixes #178
2019-08-26 17:22:21 +02:00
Xavier Morel
28bcc6b5d7 [IMP] runbot_merge: refactor some bits
* extract method to create a PR object from a github result (from the
  PR endpoint)
* move some of the remote's fixtures to a global conftest (so they can
  be reused in the forwardbot)
2019-08-26 13:53:37 +02:00
xmo-odoo
02d85ad523
[FIX] runbot_merge: less restrictive commands matching
Fixes #167 ignores casing when matching bot name
Fixes #168 ignores leading whitespace when matching commands lines
2019-08-26 13:41:33 +02:00
xmo-odoo
8b74e79da9
[FIX] runbot_merge: FF error logging message
In case of error while fast-forwarding a staging to its source, we'd
log the target to which we couldn't FF. Sadly this relied on a
`repo_name` variable which (likely since the introduction of the
"safety dance" fast forwarding) can not actually be set in case of
failure.

So stash the relevant bit (the repo name) inside the FF error exception
and use that to compose our logging message instead of a variable which
can only be None.
2019-08-26 13:41:11 +02:00
Xavier Morel
222f591deb [IMP] runbot_merge: ACL & PR name
* add missing ACL for PR feedback object
* configure name_get for PRs (which don't have a name), fixes some
  layout issues & stuff
2019-08-26 13:22:29 +02:00
Xavier Morel
0bfb018e49 [IMP] runbot_merge: table for staging history
That way all staging labels (timestamps) have the same width, and PRs
/ batches being wrapped don't look like weird-named stagings.
2019-08-26 11:56:42 +02:00
xmo-odoo
c4b7604999
[ADD] runbot_merge: staging history per branch
Closes #175
2019-08-21 14:15:10 +02:00
xmo-odoo
a84595ea04
[FIX] runbot_merge: indexing of PR objects
Github constrains a single issue (/PR) number per repository, having
different targets does not allow two PRs to share a number.

Doesn't fix but should mitigate #169 slightly.
2019-08-21 11:21:06 +02:00
xmo-odoo
429257d013
[FIX] runbot_merge: resync tags on stage change
Before this change mergebot assumes github's tags are in sync with its
"previous" state, but because tags update was highly non-atomic (one
call per removal plus one for additions) and state can further change
between a failure and an update retry (especially as the labels endpoint
fails *a lot*), it's possible for set tags (in github) to be completely
desync'd from the mergebot state, leading to very misleading on-pr
indications.

This first fetches the current tagstate from github (to not lose non-
mergebot tags) then (hopefully atomically) resets all tags tags based on
the current mergebot state. This should avoid desyncs, and eventually
resync PRs (if they change state).

Fixes #170
2019-08-21 11:17:04 +02:00
Xavier Morel
9c9b312f8a [ADD] runbot_merge: staging dependencies 2019-08-09 14:31:21 +02:00
xmo-odoo
cfc7478fcf
[FIX] runbot_merge: PR splits should be updated on PR state change
On a PR being updated, closed or unreviewed, if it is part of an
active staging that staging would get cancelled (yay). However, if the
PR was part of a pending *split*, then the split would *not* get
cancelled / updated (to remove the PR from it), and the PR could go on
to get staged as if everything were right in the world which is an
issue.

It doesn't look like it actually happened (at least I got no echo of
it), but it almost did at least once.

fixes #160
2019-07-31 09:20:02 +02:00
xmo-odoo
6cb58a322d
[IMP] runbot_merge: send feedback when approving PR which failed CI
Also add test for it & feedback of an approved PR failing CI, and fix
corner case with it (might not send a warning immediately on CI failure
depending on status requirement ordering).

Fixes #158
2019-07-31 09:19:50 +02:00
xmo-odoo
85ac2e5d5e
[IMP] runbot_merge: map PR commits to integrated commits
* when rebasing, store a map of rebased to source, that way it'll be
  possible to link cherry-picked forward ports to the originally
  integrated commit rather than just the one from the PR (which was
  likely not itself integrated as the straight merge mode is somewhat
  rare: as of 5600 PRs merged so far only 100 were straight merged)
* while at it, store the "merge head" of the PR (whether squashed,
  merged or rebased) and put *that* in the commit message

fixes #161
2019-07-31 09:19:39 +02:00
xmo-odoo
955b97a023
[IMP] runbot_merge: p=1 > split
Allows merging a fix for e.g. a common false positive during a split
but without cancelling a staging which might just pass (you never
know).
2019-07-31 09:19:28 +02:00
Christophe Simonis
5e611f54cb [IMP] runbot_merge: allow sorting & deactivating branches
Closes #144, closes #145
2019-06-28 11:31:06 +02:00
Christophe Simonis
483fc5319a [IMP] runbot_merge: show failed PR on dashboard
Closes #138
2019-06-11 11:31:24 +02:00
Xavier Morel
76ea2a4f5d [FIX] runbot_merge: ensure co-authored-by are at the bottom
Sometimes people add co-authored-by lines in the middle of their
message, where github ignores them.

Since we previously added properly handling existing (correct) C-A-B
lines in the case where we're adding fixes and signed-off-by, we might
as well fix-up existing but mispalced co-authored-by lines.

Fixes #107
2019-05-07 13:22:13 +02:00
Xavier Morel
c9f9b3050b [FIX] runbot_merge: possibly missing commit object in try_splitting
Would trigger a TypeError when trying to json.loads(False).
2019-05-07 12:53:55 +02:00
Xavier Morel
eb91e2371b [FIX] runbot_merge: check CI status when reopening a PR
Previously, creating a PR would validate the head (in case it had
already passed CI) but reopening it would not, which is inconvenient
as the CI would not automatically run on a reopened PR.

Update both the state and the head of the PR on reopen to force a
revalidation, that way if the head has already passed CI the PR will
be reopened validated and there won't be an unclear need to perform an
explicit CI run.

Fixes #119
2019-05-07 12:32:02 +02:00
Xavier Morel
b59ffc68e0 [FIX] runbot_merge: handle the bot user not being able to comment
If the author of a PR has blocked the bot user, commenting on the PR
will fail. While comment failure is technically handled in the feedback
cron, the cron will simply retry commenting on every run filling the
log with useless unactionable garbage.

Retrying is the right thing to do in the normal case (e.g. changing tags
often has transient failures), but if we figure out we're blocked we
might as well just log a warning and drop the comment on the floor, it's
unlikely the situation will resolve itself.

Couldn't test it, because the block API is a developer preview and I
just can't get it to work anyway (404 on /user/blocks even providing the
suggested media type).

And the way the block is inferred is iffy (based on an error message),
the error body doesn't seem to provide any clean / clear cut error code:

    {
        "message": "Validation Failed",
        "errors": [
            {
                "resource": "IssueComment",
                "code": "unprocessable",
                "field": "data",
                "message": "User is blocked"
            }
        ],
        "documentation_url": "https://developer.github.com/v3/issues/comments/#create-a-comment"
    }

No useful headers either.

Fixes #127
2019-05-07 10:36:53 +02:00
xmo-odoo
4206d75256
[IMP] runbot_merge: wait for (and log) repo update / staging visibility
The race condition which prompted STAGING_SLEEP rears its ugly head
again: when pushing a base repo and its dependents, it's possible for
the update to the base repo's new head to take much longer to be visible
than the dependents (or so it seems?).

In this case, CI might pick up the correct dependent but pick an older /
incorrect revision of the base, leading to a staging failing for no good
reason.

This change uses info/refs to check for the updated staging head to be
visible at the repo level after it's been set / updated via the API. It
assumes repos are in topological order.
2019-04-29 12:42:54 +02:00
Xavier Morel
aa614c6077 [IMP] runbot_merge: more reliable blocked attribute
Use the proper / actual "is there any stageable PR" query to check if
a PR is blocked as well, that way they shoudn't be diverging all the
time even if it might make PR.blocked a bit more expensive.

fixes #111
2019-04-05 08:23:56 +02:00
Xavier Morel
e12e6db653 [IMP] runbot_merge: lock commit to update its status in hook
A status being updated on a commit is a read/modify/update, meaning
it's possible for somebody else (including a concurrent event?) to
concurrently update the commit and conflict leading to the webhook
blowing up, which is undesirable as it's a data loss (whereas if it
blows up on the other side e.g. in the cron's commit processor the
cron will just take it up next iteration).
2019-03-11 14:54:58 +01:00
Xavier Morel
f5d783eb4b [FIX] runbot_merge: error in template
8d011e03d2 contains poopy bits in the
template, fix them.
2019-03-11 14:51:15 +01:00
Xavier Morel
e0320664f9 [ADD] runbot_merge: sentry integration
Might eventually extract / generalise, but for now it's simpler to
just do it in runbot_merge's post_load, that way there's no setup
change (just a small bit of configuration), and it's only enabled on
the instances runbot_merge is installed on.

fixes #97, closes #103
2019-03-07 11:56:45 +01:00
xmo-odoo
4944d6a503
[FIX] runbot_merge: small typo in error message 2019-03-06 22:50:55 +01:00
Xavier Morel
8d011e03d2 [IMP] runbot_merge: styling of awaiting and blocked lists on dashboard 2019-03-06 09:46:57 +01:00
Xavier Morel
48e08b657b [IMP] runbot_merge: send feedback on CI failure following r+
Will comment any time a statuses update folds to a CI failure on a
reviewed pull request. Might be somewhat spammy, we'll see.

No notification if the PR is not reviewed yet.

fixes #87
2019-03-05 09:03:26 +01:00
Xavier Morel
5aa9f5a567 [IMP] runbot_merge: extract commit validation to cron
Before this, impacting a commit's statuses on the relevant PR or
staging would be performed immediatly / inline with its
consumption. This, however, is problematic if we want to implement
additional processing like #87 (and possibly though probably not #52):
webhook handlers should be kept short and fast, feeding back into
github would not be acceptable.

- flag commits as needing processing instead of processing them
  immediately, this uses a partial index as it looks like the
  recommended / proper way to index a boolean column in which one of
  the values is searched much more than the other (todo: eventually
  check if that actually does anythnig)
- add a new cron for commits processing
- alter tests so they use this new cron (mostly by migrating them to
  `run_crons` though not solely as some still need more detailed
  management to properly check intermediate steps)

Fix an issue with closing a staged PR while at it (the "merging" tag
would potentially never be removed).
2019-03-05 08:07:19 +01:00
Xavier Morel
360d0e17ca [IMP] runbot_merge: don't quote signoff
Proper RFC5322 makes for much noisier messages, and seems completely
unnecessary as examples of sign-off on the internet don't quote spaces
/ names.

closes #102
2019-03-04 13:17:10 +01:00
Xavier Morel
1f30af4345 [IMP] runbot_merge: dashboard clarity
* split out truly awaiting PRs from those waiting on an event of some
  sort
* if a staging is active but doesn't have a state yet, it should be
  considered pending not cancelled

closes #74
2019-03-04 12:11:34 +01:00
Xavier Morel
b699ea7f47 [FIX] runbot_merge: validate PRs on head update
If a PR gets sync'd to a known-valid commit, it should be marked as
valid rather than get in this weird state where it's merely open but
github knows it passes CI.

Fixes #72
2019-03-04 10:34:40 +01:00