Commit Graph

1250 Commits

Author SHA1 Message Date
Xavier Morel
05444aaf3f [IMP] runbot_merge: downgrade PRs to priority 1 on r-
Before this change, `r-` on a pr[p=0] does essentially nothing. At
most it will unstage if the PR had been (somewhat unnecessarily) r+'d
in the past but then the PR will get re-staged immediately.

To avoid this odd behaviour, if r- is sent to a p=0 PR not only is
the PR unreviewed (if it was reviewed) it always gets unstaged, and
its priority gets reset to 1 (high priority but doesn't bypass CI and
review). Also send a comment on that subject so followers of the pr
are notified.

Fixes #313
2020-02-11 08:07:57 +01:00
Xavier Morel
d9661064d6 [FIX runbot_merge: prevent reopening a merged PR
Fixes #305
2020-02-11 08:07:57 +01:00
Xavier Morel
18736d282f [FIX] runbot_merge: incorrect deref'
in controller, pr is the dict of the PR coming from github, pr_obj is
the odoo-level PR object (if any)
2020-02-11 08:07:57 +01:00
Xavier Morel
289c3deee8 [ADD] runbot_merge: tests to check behaviour of github in case of updating closed PRs 2020-02-11 08:07:57 +01:00
Xavier Morel
53e46001ba [IMP] runbot_merge, forwardport: notify when main crons are off
During freezes it can be useful to notify viewers that nothing is
going to forward port or merge for a while, and that this is
intentional (not something that's broken).

Fixes #307
2020-02-11 08:07:57 +01:00
Xavier Morel
9d661480fc [IMP] runbot_merge: split staging cron in two
The staging cron was already essentially split between "check if one
of the stagings is successful (and merge it)" and "check if we should
create a staging" as these were two separate loops in the cron.

But it might be useful to disable these two operations separately
e.g. we might want to stop the creation of new staging but let the
existing stagings complete.

The actual splitting is easy but it turns out a bunch of tests were
"optimised" to only run the merge cron. Most of them didn't blow up
but it seems more prudent to fix them all.

fixes odoo/runbot#310
2020-02-11 08:07:57 +01:00
Christophe Monniez
46c8c6d635 [FIX] runbot: more flexible duplicate detection 2020-02-10 14:13:38 +01:00
Xavier Morel
c702fecda1 [FIX] runbot_merge: updating PRs in repos without required statuses
The PR creation had been fixed to always validate even without a
commit found (in case there was no need for a commit), but the update
of a PR in such a situation was not tested, and thus naturally did not
work because why would it work if it wasn't tested?

Also remove the conditional skip on updating a PR to a new head.
2020-02-07 16:11:12 +01:00
Xavier Morel
8f09eacfb5 [FIX] runbot_merge: handling of empty required_statuses for computed pr status 2020-02-07 15:54:14 +01:00
Xavier Morel
0831c899e8 [FIX] runbot_merge: the fix
The test was checking things would work properly with
required_statuses being an empty string, because I'd also forgotten an
empty field becomes stored as `False` in the database, so trying
things out live neither the PRs nor the staging would work as their
assumption that they could straight split the required_statuses would
always fail.

Update the test to better match expectations, and hopefully this is
the end of that saga.
2020-02-07 09:55:22 +01:00
Xavier Morel
4bdf7e5eda [FIX] runbot_merge: stagings involving repos w/o required statuses
PRs transitioning to 'ready' had been checked and tested but turns out
I had completely forgotten to test that stagings would validate
properly therefore of course they didn't.

The issue here was I'd forgotten `''.split(',')` returns `['']` rather
than `[]`, so on an empty required_statuses the staging validator
would keep looking for a status matching the context `''` and would
never find it, keeping the staging pending until timeout. So most
likely the problem could have been resolved by just adding a condition
to

    [r.strip() for r in repomap[c.sha].required_statuses.split(',')]

but I'd already done all the rest of the reorganisation by that point,
test pass and I think it's a somewhat better logic. Therefore I'll go
with that for now.

* properly handle empty required_statuses during staging validation
* remove the final postcondition, if we're missing commits which don't
  require any statuse we should not care
* expand test to include up to merging PRs
* automatically create dummy commits when creating stagings, that way
  the relevant commits are in the database (can't hurt)

PS: an other alternative would have been to filter out or skip ahead
on commits which don't require any statuses aka cmap &
required_statuse / cmap would not even have that entry
2020-02-07 08:29:55 +01:00
Xavier Morel
fd24b791b3 [FIX] forwardport: prefix of working copy when forward porting
The missing dash makes it hard to differentiate the target's name and
the random crap TemporaryDirectory tacks on. Add a separator dash.
2020-02-07 08:29:55 +01:00
Xavier-Do
6909b7e583 [IMP] runbot: allow build copy 2020-02-05 15:59:57 +01:00
Xavier-Do
c561f212e2 [IMP] runbot: improve build form view 2020-02-05 15:59:57 +01:00
Xavier-Do
a7ecf6a631 [FIX] runbot: fix build creation from view form
Before this commit, creating branch/build from view form may lead to errors
because of missing fields/ inexisting id.
2020-02-05 15:59:57 +01:00
Xavier-Do
3345d18050 [IMP] runbot: split prending count and scheduled count 2020-02-04 14:35:07 +01:00
Xavier Morel
f4fd17a884 [ADD] runbot_merge: sequence on repositories 2020-02-04 07:45:52 +01:00
Xavier Morel
c7393e2da9 [IMP] runbot_merge: explain why a PR is blocked on the dashboard
Refactor the selection thingie, hopefully in a way which doesn't
absolutely crater performances, so that it's possible to explain the
reason why a PR is considered blocked.
2020-01-31 15:19:32 +01:00
Xavier Morel
35dbfd2c12 [FIX] runbot_merge: status deduplication (maybe)
Despite the existing dedup' sometimes the "xxx failed on this
forward-port PR" would still get multiplicated due to split builds
e.g. in odoo/odoo#43935 4 such messages appear within ~5 minutes, then
one more 10mn later.

This is despite all of them having the same "build" (target_url) and
status (failure). Since the description is the only thing that's not
logged I assume that's the field which varies and makes the dedup'
fail. Therefore:

* add the description to the logging (when getting a status ping)
* exclude the description when checking if a new status should be
  taken in account or ignored: the build (and thus url) should change
  on rebuild

Hopefully fixes #281
2020-01-31 10:40:59 +01:00
Christophe Monniez
e267533513 [IMP] runbot: add pdfminer to Docker image
As pdfminer is going to be used in Odoo attachement_indexation module and
that there is no python3-pdfminer package in Ubuntu Bionic, it was
decided to have an optional dependency.

In order to run the attachement_indexation tests, the pip package is
added to the runbot Docker image.
2020-01-30 14:52:29 +01:00
Xavier Morel
3f61dc9ce4 [IMP] forwardport: warning message when co-dep PR has a conflict
* Add some more information as to why the user *should* do on the PR
  the message is printed on, the previous message left that to their
  imagination
* The PR selection was *completely* wrong as it would select the old
  PRs which really isn't what we want. And turns out there's no good
  reason to create & send the feedback in the loop creating the
  forward-port prs, that can be moved to a followup loop where we have
  created hopefully created all the forward-port PRs.

  Also technically we could do even better than currently and remap
  the prs mapped to conflict data to the new PRs and know exactly
  which of the forward-ported PRs is faulty, but that seems overkill
  for now.
2020-01-30 13:56:01 +01:00
Xavier Morel
e065b7aba7 [FIX] forwardport: reintroduce final newline in a comment
Tests check for it, easier to fix one line than change all the
relevant tests.
2020-01-30 09:41:16 +01:00
Xavier Morel
9aac1b4a3e [ADD] forwardport: special handling of adding branches to projects
If a new branch is added to a project, there's an issue with *ongoing*
forward ports (forward ports which were not merged before the branch
was forked from an existing one): the new branch gets "skipped" and
might be missing some fixes until those are noticed and backported.

This commit hooks into updating projects to try and see if the update
consists of adding a branch inside the sequence, in which case it
tries to find the FP sequences to update and queues up new
"intermediate" forward ports to insert into the existing sequences.

Note: had to increase the cron socket limit to 2mn as 1mn blew up the
big staging cron in the test (after all the forward-port PRs are
approved).

Fixes #262

[FIX]
2020-01-29 15:59:43 +01:00
Xavier Morel
0d33b87579 [FIX] finally make pytest work properly w/ model proxy
At some point pytest added support for dataclass & attrs introspection
by looking up some specific meta-fields when trying to format
objects (after an assertion fails).

It tries to access the attributes, falls back to something else if it
gets an AttributeError but apparently falls over if it gets something
else, which is what'd happen here as read() would generate a
ValueError which would get re-raised as-is on the client
side.. However pytest doesn't really make the issue clear, and the
logging from RPC likely got lost in the noise from the github logging.

The fix is to simply convert errors from read() into proper
AttributeError. And blacklist fields we know make no sense to avoid
confusing tracebacks in the log.
2020-01-29 15:59:43 +01:00
Xavier Morel
43e6f5e5ec [FIX] forwardport: logging.warn -> logging.warning
logging.warn is deprecated but I've a hard time shaking the habit.
2020-01-29 15:59:43 +01:00
Xavier Morel
bb2582ec29 [IMP] mergebot: allow running tests without verbose github logging
Mergebot & forwardbot have ultra-verbose logging of all github
interactions in order to better understand what happens exactly when
there are issues with gh integration (and/or provide to GH support).

However in most cases this is a pain in the ass when reviewing test
logs. So suppress these github_requests logs by default when testing.
2020-01-29 15:59:43 +01:00
Xavier Morel
1b9bd67776 [FIX] runbot_merge: incorrectly logged PRs
A while back I implemented name_get/display_name to print PRs using
the canonical github format (owner/repo#number), however looks like
some of the logging calls were still using bespoke formatting.
2020-01-29 15:59:43 +01:00
Xavier Morel
a0fe545d86 [IMP] forwardport: warn when linked PR failed to FW
Before this change, if multiple co-dependent PRs get forward-ported
and one of them has a conflict the notice on the others is very
limited: they're tagged as `conflict` but there is no other
information provided in the PR description or in the subsequent
message.

Add a small warning to these other PRs, for clarity.

Closes #302
2020-01-29 15:55:06 +01:00
Xavier Morel
5a8f821de0 [FIX] forwardport: recover from failure to create forward port PR
Currently if the creation of a forward-port pull request fails:

* the branches are left un-cleaned
* preceding PRs are left open
* the PR whose creation failed may or may not have actually failed,
  and may or may not still be open

We need to delete the forward port branches anyway, and IIRC
that *should* automatically close the PR. Sadly making it so github
predictably / reliably blows up when trying to create a PR via the API
is difficult so this is essentially untestable.

Closes #296
2020-01-29 15:43:57 +01:00
Xavier Morel
5fae01e53e [IMP] forwardport: feedback on commands
The forwardbot's command parsing was missing feedback when trying to
use commands without the proper ACL. This would make some situations
of comments seemingly being lost hard to diagnose.

Closes #300
2020-01-29 15:09:09 +01:00
Xavier Morel
ecd75b1822 [FIX] runbot_merge: statuses migration script should be for 13.0
Moving statuses from project to repo was originally developed on 11,
but since the PR was only merged after the 13.0 update, the script
migration script should be moved to match.
2020-01-29 13:29:21 +01:00
Xavier Morel
60574d6fe2 [ADD] forwardport: support for branch filtering
Add handling of branch filtering to the forwardport module:

* don't forward port (and trigger an error) when trying to port
  PRs to different next targets
* otherwise port normally

e.g. given a project with repos A and B and branches a, b and c, with
branch b being excluded from repo B:

* a PR merged into A.a will be forward-ported to A.b and A.c
* a PR merged into B.a will be forward-ported to B.c (skipping the
  excluded B.b)
* a PR set merged into (A.a, B.a) will *not* be forward-ported, and a
  message will be posted to each PR denoting the incompatibility
2020-01-24 13:39:14 +01:00
Xavier Morel
dd22f687bf [IMP] runbot_merge: allow filtering out branches from repositories 2020-01-24 13:39:14 +01:00
Xavier Morel
6b3c81a177 [FIX] make pytest cross-module-runnable
The pytest suite had been partially unified between mergebot and
forwardport but because of session-scoped modules it could not run
across those.

Make the db cache lazy and able to cache multiple databases, and move
the "current required module" to function scoped, this way things
should (and seem to) work properly on runs involving mergebot & fwbot.

Next step: xdist! (need to randomise repo names for that, probably).
2020-01-24 13:39:14 +01:00
Xavier Morel
7dfa973b57 [IMP] runbot_merge, forwardport: move required statuses to repository
Allows more flexibility in project composition as different
repositories can each have their own CI passes.
2020-01-24 13:39:14 +01:00
Xavier Morel
b2f9bd697c [FIX] runbot_merge: testsuite compatibility with pytest-xdist
randomise the name of the repositories created so they don't collide
and lead to odd results when running concurrent test cases which
specify the same repo name (a common property).

As a result, ignore the "no delete" flag for creation: there should be
no way to land on a pre-existing repo name even if we didn't clean
them up.

Also stagger the check of a running ngrok process: when pytest starts
its worker processes, all workers will run the tunnel fixture, and
since the ngrok process takes some time to get into a stable run state
chances are multiple workers will fail to connect and try to start
ngrok concurrently, which blows up as ngrok just kills the extra
processes instead of merging / proxying into an existing session. A
proper lockfile would probably be better but...

Fixes #297
2020-01-24 13:36:05 +01:00
Denis Ledoux
d0138712bd [ADD] runbot_merge: automatic reviewer de-provisioning
When an employee sadly leaves Odoo,
the Odoo production database (odoo.com) will call these routes
in order to remove the reviewer rights automatically.

So a user who no longer works for Odoo can't "r+" Github PRs.

This is related to odoo/internal#617
2020-01-24 13:28:18 +01:00
Xavier-Do
e65859c161 [FIX] runbot: catch ValueError instead of RunbotException in safe_eval 2020-01-24 13:09:36 +01:00
Xavier Morel
4bd65a4dff [MERGE] v13 migration of mergebot/forwardbot 2020-01-23 16:35:10 +01:00
Xavier Morel
5dccb141b7 [FIX] runbot_merge: disable homepage added enabled by website
Pages take over from redirections which really is a pain in the ass
when trying to find out why the bloody redirection seemingly refuses
to work.

Note: can't use the record tag because homepage_page is marked as
noupdate, so we have to bypass the flag checking.
2020-01-23 13:57:33 +01:00
Xavier Morel
b1ce1e82e0 [REM] runbot_merge: str override on pull_request
Interaction of CacheMiss and BaseModel is fucked, leading to an
infinite loop when trying to provide useful __str__ on a model (by
accessing model fields).
2020-01-23 10:08:55 +01:00
Christophe Monniez
edda6c0265 [IMP] runbot: build docker image once per loop turn
At this moment, the Docker image is built at the beginning of each
runbot build. This blocks the _scheduler while the image is built.

With this commit, the image is built before calling the _scheduler and
is not linked to a runbot build.

Also, the necessary dirs are created in the static path before starting
the loop.
2020-01-22 13:50:01 +01:00
Christophe Monniez
61101eaf46 [FIX] runbot: fix separtor class in build template 2020-01-22 13:32:27 +01:00
Christophe Monniez
e4e2634de1 [FIX] runbot: keep tests directory
Since 857821e4 a screenshot can be saved in the build directory under
the "tests" subdirectory.
Unfortunately, this directory is cleaned in case of local_cleanup.

With this commit, the 'tests' subdirectory is kept in place.
2020-01-22 13:32:27 +01:00
Xavier Morel
3f44bb5c9e [FIX] runbot_merge: styling crap
bs4 yields complete vomit on the template as-is (see:
https://imgur.com/a/XIMn7MX).

Add a bunch of color and styling overrides to get something closer to
the original, and move the existing styles to a "proper" scss file
while at it.
2020-01-21 11:21:14 +01:00
Xavier Morel
5d1772a9a5 [FIX] runbot: redirect to the relevant build on actions
Currently killing, waking or rebuilding a build then redirects to the
repository root which is worse than useless: you've lost the build you
come from, and for rebuilds it's no help getting to the new build.

Since it's very easy to go from a build to its repository, redirect to
the "most useful" build instead:

* if rebuilding the current build (from its page), open the page of
  the new build which was just created
* otherwise reload the current page whatever it is
2020-01-21 09:21:02 +01:00
Xavier-Do
4ae6907e3a [FIX] runbot: keep description on exact rebuild 2020-01-20 14:44:16 +01:00
Christophe Monniez
ca4768230d [FIX] runbot: avoid crash when creating migration symlink 2020-01-17 17:14:12 +01:00
Christophe Monniez
bf9ec0b7db [FIX] runbot: add file command to build Odoo 8.0 and 9.0 2020-01-17 17:10:25 +01:00
Xavier-Do
eede4dcd77 [IMP] runbot: filter hidden on monitoring page 2020-01-17 16:36:30 +01:00