Commit Graph

1971 Commits

Author SHA1 Message Date
Xavier-Do
34e18cf797 [FIX] runbt: runbot build error parse_log fixes 2024-11-19 09:30:42 +01:00
Christophe Monniez
c9e8e13606 [FIX] runbot: fix frontend team dashboards 2024-11-19 08:38:08 +01:00
Xavier Morel
4563fc5fc0 [FIX] runbot_merge: dashboard image is branch created via SQL
If a branch is created via SQL directly, one might forget to set its
`write_date`, leading the computation of the `last_modified` to be
*really* unhappy as it's asked to compare a bunch of datetime objects
to `False` in order to compute the `max`.

Substitute missing `write_date` with `datetime.min` so they are
extremely unlikely to influence the computation until and unless the
branch gets updated.
2024-11-18 14:52:15 +01:00
Xavier Morel
63a0ee90b2 [ADD] runbot_merge: views from custom
Add a few views / view extensions set as custom on the production
mergebot which I never remembered to implement in the actual source.
2024-11-18 14:45:21 +01:00
Xavier Morel
3fe29ba8f6 [IMP] forwardport: batch list
Since b45ecf08f9 forwardport batches
which fail have a delay set in order to avoid spamming. However that
delay was not displayed anywhere, which made things confusing as the
batch would not get run even after creating new triggers.

Show the delay if it's set (to a value later than now), as a relative
delta for clarity (as normally the delay is in minutes so a full blown
date is difficult to read / aprehend), and allow viewing and setting
it in the form view.

Fixes #982
2024-11-18 14:18:25 +01:00
Xavier Morel
fbfb96be53 [IMP] runbot_merge: ping commenter when fetching PR due to comment
If a comment causes an unknown PR to be fetched, it's a bit odd to
ping the author (and possibly reviewer) anyway as they're not super
concerned (and technically we could be ignoring the purported /
attempted reviewer).

So if a fetch job was created because of a comment, remember the
comment author and ping *them* instead of using the default ping
policy.

Fixes #981
2024-11-18 13:52:27 +01:00
Xavier Morel
c974f51036 [IMP] runbot_merge: trigger staging if re-enabled for branch
If staging gets re-enabled on a branch (or the branch itself gets
re-enabled), immediately run a staging cron as there may already be
PRs waiting, and no trigger enqueued: cron triggers have no payload,
they just get removed when the cron runs which means if a bunch of PRs
become ready for branch B with staging disabled, the cron is going to
run, it's going to stage nothing on that branch (because staging is
disabled) then it's going to delete all the triggers.

Fixes #979
2024-11-18 13:09:23 +01:00
Xavier Morel
5441ba12ae [FIX] runbot_merge: format_patch if --no-prefix
Turns out you can configure format-patch with `--no-prefix` and some
people (*cough cough* mat) have that in their standard setup, so the
assumption of needing to strip 1 level of prefix does not necessarily
hold.

Also fix a few more issues:

- some people (*cough cough* still mat) also use `-n` by default,
  which adds the series sequence (`n/m`) even for a single patch,
  handle that correctly
- logging patch application errors is pretty useful when patching
  fails and I'm trying to get the information via logs, do that
- especially when I decide to add error messages to tracking *but
  forgot to show the chatter by default*, fix that as well

The commit-based patcher worked first try, and patch-based would have
worked too if not for those meddling kids. In the future it might be a
good idea to reify the stripping level (`-p`) on the patch object
though, and maybe provide a computed preview of the list of files to
patch, so issues are easier for the operator to diagnose.
2024-11-18 12:37:44 +01:00
Xavier Morel
a12e593fba [FIX] runbot_merge: backport wizard
- fix incorrect view specs (the action id comes first)
- add a wizard form and hook it into the PR, completely forgot to do
  that
- usability improvements: filter branches to be in the same project as
  the PR being backported, and older than the current PR's branch

The latter is a somewhat incomplete condition: ideally we'd want to
only allow selecting branches preceding the target of the *source* of
the PR being backported, that way we don't risk errors when
backporting forward-ports (the condition should be checked in the
final action but still).

Also we're only filtering by sequence, so we're missing the name part
of the ordering, hence if multiple branches have the same sequence we
may not allow selecting some of the preceding branches.
2024-11-18 09:48:48 +01:00
William Braeckman
c2e9aaf387 [FIX] runbot: fix typo in server action 2024-11-14 12:05:10 +01:00
Christophe Monniez
9151c26232 [FIX] runbot: fix invalid field name 2024-11-14 09:33:44 +01:00
Christophe Monniez
2697a28e54 [IMP] runbot: allow override global docker registry on a host 2024-11-14 09:11:26 +01:00
Christophe Monniez
3197f75e45 [FIX] runbot: remove bulk update button from error content tree 2024-11-14 08:43:06 +01:00
Christophe Monniez
dc7eb66903 [IMP] runbot: add a display field to show error id in content tree 2024-11-14 08:43:06 +01:00
Christophe Monniez
b95256155c [IMP] runbot: add a default filter on build errors 2024-11-14 08:43:06 +01:00
Christophe Monniez
23be10f75b [IMP] runbot: add a search panel on build error list view 2024-11-14 08:43:06 +01:00
Christophe Monniez
90f114c730 [IMP] runbot: add a default filter on error content tree 2024-11-14 08:43:06 +01:00
Christophe Monniez
a6823f6d53 [FIX] runbot: adapt migration script
Fix a failure when a build error is not linked to any build. It happens
when a build error was merged into another.
2024-11-14 08:43:06 +01:00
Xavier-Do
56e242a660 [IMP] runbot: refactor build error models
The initial idea to link an error to another one was a quick solution
to group them if they where related, but this became challenging
to copute metada regarding errors.

- The displayed error message was not always consistent with the real
root cause/the error that lead here.
- The aggregates (lets says, linked buils ids) could be the one of the
error, or from all error messages. Same for the versions, first seen, ..
This is confusing to knwo what is the leist we are managing and what is
the expecte result to display

Main motivation:
on a standard error page (will be changed to "assignment"), we want to
have the list of error message that is related to this one. We want to
know for each message (a real build error) what is the version,
first seen, ...
This will give more flexibility on the display,

The assigned person/team/test-tags, ... are moved to this model
The appearance data remains on the build error but are aggregate on the
assignation.
2024-11-14 08:43:06 +01:00
Xavier-Do
d990b39258 [FIX] runbot: consider to_upgrade as to_upgrade_to 2024-11-12 14:01:05 +01:00
Christophe Monniez
7e01b711ad [IMP] runbot: add charset to content-type
This commit adds the utf-8 Content-Type to nginx response headers for
txt files. That way, the logs files can be properly viewed in browsers.
2024-11-12 12:14:40 +01:00
Christophe Monniez
5b70e91043 [IMP] runbot: add a searchpanel on bundle search 2024-11-12 12:14:12 +01:00
Xavier-Do
2b690de566 [IMP] runbot: allow to disable upgrade from a version 2024-11-12 08:33:14 +01:00
Christophe Monniez
ecd9681b65 [FIX] runbot: properly join log args
When a type error occurs when trying to format a message in a build log,
the suspicious are are joined with the message. But as the args may be a
tuple, an errors occurs when concatenating the message with the args
during the join.

With this commit, we ensure that the args are casted into a list.
2024-11-09 17:42:52 +01:00
William Braeckman
63dac316ab [FIX] runbot: remove console.log 2024-11-06 16:32:35 +01:00
Xavier Morel
5b94dcce35 [FIX] runbot_merge: reset markdown renderer
pymarkdown's footnotes plugin *saves footnotes across invocations by
default*. Even if I understand the documented use case it seems wild
that it's not opt-in...

Anyway disable that resetting all internal state. Thanks rfr for the
inital report that things were looking odd.
2024-10-29 13:13:59 +01:00
Xavier Morel
11f2231e82 [FIX] runbot_merge: missing model description 2024-10-22 15:08:00 +02:00
Xavier Morel
e7716f8b77 [FIX] *: fw=no reflection in the PR dashboard
Like limit, fw=no should restrict the table length, in this case to
just the current branch (as we're not forward porting at all).

Before this, `no` would not be applied as a limit visually, the table
would still go up to the main branch which is very confusing.

Fixes #962
2024-10-22 15:05:48 +02:00
Xavier Morel
2fea318830 [IMP] runbot_merge: hide concurrent update errors
As far as I can tell they are properly handled:

- In `handle_status` we let the http layer retry the query, which
  pretty much always succeeds.
- In `Commit.notify`, we rollback the application of the current
  commit, meaning it'll be processed by the next run of the cron,
  which also seems to succeed every time (that is going through the
  log I pretty much never notice the same commit being serialization
  failure'd twice in a row).

  Which we can trigger for faster action, this last item is not
  entirely necessary as statuses should generally come in fast and
  especially if we have concurrency errors, but it can't hurt.

This means the only genuine issue is... sql_db logging a "bad query"
every time there's a serialization failure.

In `handle_status`, just suppress the message outright, if there's an
error other than serialization the http / dispatch layer should catch
and log it.

In `Commit._notify` things are slightly more difficult as the execute
is implicit (`flush` -> `_write` -> `execute`) so we can't pass the
flag by parameter. One option would be to set and unset
`_default_log_exception`, but it would either be a bit dodgy or it
would require using a context manager and increasing the indentation
level (or using a custom context manager).

Instead just `mute_logger` the fucking thing. It's a bit brutish and
mostly used in tests, but not just, and feels like the least bad
option here...

Closes #805
2024-10-22 14:12:04 +02:00
Xavier Morel
2174d7da31 [IMP] runbot_merge: optimise edited event
For the longest time Github's `change` key was borked when
transitioning a description to and from empty. They fixed that during
2023, which I already saw and impacted on
DC (xmo-odoo/dummy_central@1ebed9d418
and xmo-odoo/dummy_central@937e87c2a4)
but this had yet to be taken in account by the mergebot.

This is now done, the code is functionally reverted to what it was
before I realised `changes` was hosed and moved off of it in
3da1874196.

Fixes #743
2024-10-22 13:12:28 +02:00
Xavier Morel
cf4d162907 [ADD] *: PR backport wizard
This is not a full user-driven backport thingie for now, just one
admins can use to facilitate thing and debug issues with the
system. May eventually graduate to a frontend feature.

Fixes #925
2024-10-22 11:41:58 +02:00
Xavier Morel
ed1f084c4f [IMP] runbot_merge: style fixes
- replace manual token_urlsafe by actual token_urlsafe
- make conditional right side up and more readable
- replace match by fullmatch, should not change anything since we end
  with a greedy universal match but is slightly more explicit
2024-10-22 10:51:47 +02:00
Xavier Morel
d9e6d39448 [IMP] mergebot_test_utils: minor style fixes 2024-10-22 10:51:30 +02:00
Xavier Morel
632763d390 [CHG] runbot_merge: move labels cron to triggered
Missed it during the previous pass, probably because it's in the
middle of `pull_requests.py`. It's a classic template for triggered
crons since the model is just a queue of actions for the cron.
2024-10-22 10:50:09 +02:00
William Braeckman
2fec54838e [IMP] runbot: add customer is me filter
As we try to assign ourselves as customer to build errors it is useful
to add a new filter to find errors on which we are the customer more
easily.
2024-10-22 09:25:53 +02:00
Xavier Morel
640392dc20 [FIX] significantly speed up local testing
The mergebot tests have always been pretty gentle on system load which
is nice, however it's just looking at the list of longest tests that I
realised / re-membered the hook wait duration is 10 seconds for the
benefit of github, which doesn't really matter locally. This means on
interaction / cron-heavy tests the test might only be using on the
order of 10% CPU or something, that is a waste of time.

TBF this is easily compensated by increasing the concurrency of the
test suite (e.g. from 16 to 32 when I switched machine, but it seems
as if not more sensible to lower the webhook wait delay to something
more reasonable. 1s seems to be a good fit here, on my new computer at
n=16 it leads to the test suite running in 15mn at 600% CPU (which is
pretty good on a 6/12 CPU as it loads the system heavily but doesn't
completely bog it down).

Reducing it to 0.5s, the test suite takes the same duration but CPU
load increases to 770%, and errors creep up, likely a mix of
concurrency issues in the DB and dummy-central sending webhooks too
slowly as we compete with it for CPU resources (could actually make
sense to restrict the number of threads tokio can use). Reducing
concurrency could make this work better, but I think at this point
we're in a pretty good state, it's even somewhat reasonable to run the
test suite sequentially (taking about 1h10 but being functionally
invisible in terms of load).
2024-10-18 10:19:28 +02:00
Xavier Morel
5748c086e5 [FIX] runbot_merge: status of closed PRs in extant batches
If a PR is closed but part of an ongoing batch, the change in status
of the batch might be reflected on the PR still:

- if a PR is closed and the batch gets staged, the PR shows up as
  being staged
- if the PR is merged then the batch gets merged, the PR shows up as
  merged

Fixes #914

Also remove the unused `_tagstate` helper property.
2024-10-16 12:17:30 +02:00
Christophe Monniez
dda0170a89 [FIX] runbot: avoid trying to push unexisting image
When an image is build for the first time and the build fails, an
ImageNotfound Exception is raised when trying to push the image.

Whith this commit, a warning is emmited instead of crashing in such a
case.
2024-10-10 09:16:56 +02:00
Christophe Monniez
a8b3d6e205 [FIX] runbot: avoid copying always_pull field
This commit will prevent people acting like cowboys to break the whole
system by duplicating a Dockerfile without disabling the always_pull
field.
2024-10-10 07:41:04 +02:00
Xavier-Do
ad87096436 [IMP] runbot: allow to skip requirements from trigger 2024-10-08 10:47:48 +02:00
Xavier Morel
20a4e97b05 [CHG] runbot_merge: make merge method non-blocking
Because of the false negatives due to github's reordering of events on
retargeting, blocking merge methods can be rather frustrating or the
user as what's happening and how to solve it isn't clear in that case.

Keep the warnings and all, but remove the blocking factor: if a PR
doesn't have a merge method and is not single-commit, just skip it on
staging. This way, PRs which are actually single-commit will stage
fine even if the mergebot thinks they shouldn't be.

Fixes #957
2024-10-07 08:07:59 +02:00
Xavier Morel
4215be770d [IMP] runbot_merge: move read_tracking_value to utils
And use it in test_trivial_flow instead of the kinda half-assed manual
version.
2024-10-07 08:06:03 +02:00
Xavier Morel
a45db1e089 [IMP] conftest: support for a more generic current_date 2024-10-07 08:04:25 +02:00
Xavier Morel
3a8b4684da [IMP] conftest: support for mapped(fn) 2024-10-07 08:03:36 +02:00
Xavier Morel
6a1b77b92c [ADD] runbot_merge: support for unstaged patches
Unstaged changes can be useful or necessary for some tasks
e.g. absolute emergency (where even faking the state of a staging is
not really desirable, if that's even possible anymore), or changes
which are so broad they're difficult to stage (e.g. t10s updates).

Add a new object which serves as a queue for patch to direct-apply,
with support for either text patches (udiff style out of git show or
format-patch) or commits to cherry-pick. In the former case, the part
of the show / format-patch before the diff itself is used for the
commit metadata (author, committer, dates, message) whereas for the
commit version the commit itself is reused as-is.

Applied patches are simply disabled for traceability.

Fixes #926
2024-10-03 12:06:00 +02:00
Xavier Morel
aac987f2bb [FIX] runbot_merge: dashboard display nits
- fix staging reasons containing escaped quotes (would render as
  ` ` to the end user)
- remove extra spacing around PR title @title/popovers
- simplify a few view conditionals through judicious use of `t-elif`
  and nesting
- make `staging_end` non-computed as it's not computed anymore, just
  set if and when the staging gets disabled
  (146564a90a)
2024-09-27 14:13:43 +02:00
Xavier Morel
430ccab2cb [IMP] runbot_merge: suppress view validation warning
This is a dumb false positive, kill it.
2024-09-27 12:53:51 +02:00
Xavier Morel
8f27773f8d [IMP] forwardport: surfacing of modify/delete conflicts
Given branch A, and branch B forked from it. If B removes a file which
a PR to A later modifies, on forward port Git generates a
modify/delete conflict (as in one side modifies a file which the
other deleted).

So far so good, except while it does notify the caller of the issue
the modified file is just dumped as-is into the working copy (or
commit), which essentially resurrects it.

This is an issue, *especially* as the file is already part of a
commit (rather tan just a U local file), if the developer fixes the
conflict "in place" rather than re-doing the forward-port from scratch
it's easy to miss the reincarnated file (and possibly the changes that
were part of it too), which at best leaves parasitic dead code in the
working copy. There is also no easy way for the runbot to check it as
adding unimported standalone files while rare is not unknown
e.g. utility scripts (to say nothing of JS where we can't really track
the usages / imports at all).

To resolve this issue, during conflict generation post-process
modify/delete to insert artificial conflict markers, the file should
be syntactically invalid so linters / checkers should complain, and
the minimal check has a step looking for conflict markers which should
fire and prevent merging the PR.

Fixes #896
2024-09-27 12:37:49 +02:00
Xavier Morel
ef22529620 [ADD] support for recursive tree to the test GH proxy 2024-09-27 12:36:50 +02:00
Xavier Morel
26882c42aa [FIX] warning in test logs 2024-09-27 12:36:02 +02:00