The main motivation is to allow to create params from data
the "new" method was called with a value list instead of a dict.
Also, makes it possible to update params when the registry is not laoded
The initial motivation is to remove the flush when a log_counter is
written. This flush was initially usefull when the limit was in a
psql trigger, but finally add a side effect to flush everything before
starting the docker. This was limiting concurrent update after starting
the docker, but we still have no garantee that the transaction is
commited after starting the docker. The use case where the docker is
started but the transaction is not commited was not handled well and was
leading to an infinite loop of trying to start a docker (while the
docker was already started)
This refactoring returns the docker to the scheduler so that the
schedulter can commit before starting the docker.
To achieve this, it is ideal to have only one method that could return
a callable in the _scheduler loop. This is done by removing the run_job
from the init_pending method. All satellite method like make result
are also modified and adapted to make direct write: the old way was
technical debt, useless optimization from pre-v13.
Other piece of code are moved arround to prepare for future changes,
mainly to make the last commit easier to revert if needed.
[FIX] runbot: adapt tests to previous refactoring
Trying to log when the transaction is in error is useless and create
noise in the logs.
Flushing is also useless there now that we have the local logs,
and it makes the error confusing since the error does not come from the
log_counter update but from the update of the global state on the
parents global_results.
Currently, the addons_path is intuited from the location of the script, with the assumption that the only custom addons path will be runbot itself.
This commit introduces an addons_path parameter to support custom deployments. If the parameter is not set, the existing behaviour is unchanged.
When parenting a build error, if a test_tag is set on it, the tag is
transferred to the parent and cleared to an empty string.
In that case, a single `-` appears in the disabling tags and leads to an
apocalyptic situation ... the runbot builds don't not run any tests.
With this commit, the test_tags is set to `False`.
Since removeprefix was not available in ubuntu 20.04, a easier alternative
using rebase was used.
The initial assumption was that the prefix would look like `odoo/addons/`
and won't be in the filename.
When the repo is enterprise, the prefix is `enterprise/` meaning that
module name ending with `enterprise` will be truncated
`repo._get_module('enterprise/mail_enterprise/static/src/widgets/form_renderer/form_renderer.js')`
will output
`mail_static`
The loggers would only print the "tail" of the path, not including the
repo name, or the `/repos` prefix.
While this made logs shorter, it was not intentional and made
debugging some issues on endpoints harder than necessary as the calls
had to be adjusted mentally, which is completely unnecessary.
1cea247e6c tried to improve staging
checks to avoid staging PRs in the wrong state, however it had two
issues:
PR state
--------
The process would reset the PR's state to open, but unless the head
was being resync'd it wouldn't re-apply the statuses on the state,
leading to a PR with all-valid statuses, but a missing CI.
Message
-------
The message check didn't compose the PR message the same way PR
creation / update did (it did not trim the title and description
individually, only after concatenation), resulting in a
not-actually-existing divergence getting signaled in the case where
the PR title ends or the description starts with whitespace.
Expand relevant test, add a utility function to compose a PR message
and use it everywhere for coherence.
Also update the logging and reporting to show a diff of all the
updated items (hidden behind a `details` element).
Seems to be a new github weirdness: when forking a repo, when hitting
it fast enough it's apparently possible to see the repository in an
incomplete state (without its content).
Obviously this causes tests to fail.
Complete the post-fork testing by listing the branches of the fork,
and considering the repo created once we see a non-empty list (the
source repo should always have at least one branch, which should be
copied over when forking).
If there are bump PRs anyway: the bump commits will cause the
forward-port of the staging to fail, so might as well clearly notify
everybody of the issue if there is a pending staging, and not waste
too much time waiting for a staging which can not succeed.
We could also cancel stagings when there's no bump PR, but it's not
clear that there's any reason to do so: if we didn't touch any master
branch, there's no reason for the staging to fail, or to otherwise
cancel it.
And obviously we can't have staged anything on the new branch so
there's nothing to cancel.
Part-Of: #718
I DECLARE BANKRUPTCY!!!
The previous implementation of labels lookup was really not
intuitive (it was just a char field, and matched labels by equality
including the owner tag), and was also full of broken edge
cases (e.g. traceback if a label matched multiple PRs in the same repo
because people reuse branch names).
Tried messing about with contextual `display_name` and `name_search`
on PRs but the client goes wonky in that case, and there is no clean
autocomplete for non-relational fields.
So created a view which reifies labels, and that can be used as the
basis for our search. It doesn't have to be maintained by hand, can be
searched somewhat flexibly, we can add new view fields in the future
if desirable, and it seems to work fine providing a nice
understandable UX, with the reliability of using a normal Odoo model
the normal way.
Also fixed the handling of bump PRs, clearly clearing the entire field
before trying to update existing records (even with a link_to
inbetween) is not the web client's fancy, re-selecting the current
label would just empty the thing entirely.
So use a two-step process slightly closer to the release PRs instead:
- first update or delete the existing bump PRs
- then add the new ones
The second part is because bump PRs are somewhat less critical than
release, so it can be a bit more DWIM compared to the more deliberate
process of release PRs where first the list of repositories involved
has to be set up just so, then the PRs can be filled in each of them.
Fixes#697
In order to support partial freezing, we need the ability to remove
some of the release lines for the repos we don't want to
freeze (e.g. because they don't use per-version branches).
This subsequently means we need the ability to *create* new lines if
we fucked up and removed one we should not have. Alternatively the
freeze meat-bot could cancel the entire thing and redo the wizard but
that seems harsh and mean, so don't do that.
Fixes 0f3647b7c7 which specifically
mentioned partial freeze then proceeded to make them entirely
impossible anyway.
Part of #718
Previously the mergebot would only sync the head commit, but synching
more is useful.
Also update the final sanity check on staging:
- as with check, update the message & target branch
- reset PR state and post a message when updating message instead of
doing so silently
Note: maybe only fail the staging if the message is updated *and*
relevant to staging (aka there's a merge method and it's not
`rebase`)?
Fixes#680
Currently, if a PR forward-port PR gets detached the reason for it is
not always obvious, and may have to be hunted in the logs or in
"sibling" PRs.
By writing a forward port reason (hopefully) ever time we detach a PR,
and displaying that reason in the form and dashboard, the
justification should be a lot more obvious.
Fixes#679
When inserting a new branch, if there are extant forward ports
sequences spanning over the insertion, new forward ports must be
created for the new branch.
This was the case, *however* one of the cases was handled incorrectly:
PR batches (PRs to different repositories staged together) must be
forward-ported as batches. However when creating the "insert" batches,
these PRs would be split apart, leading to either inconsistent states
or an inability to merge the new forward ports (if they are
co-dependent which is a common case). This is because the handler
would look for all the relevant PRs to forward ports, but it would
fail to group them by label thereafter.
Fix that, create the `insert` batches correctly, and augment the
relevant test with an additional repository to test this case.
No gh issue was created, but this problem was reported on
odoo/odoo#110029 and odoo/enterprise#35838 (they were re-linked by
hand in the runbot and mergebot, but on github the labels remain
visibly different, one having the slug ZCwE while the other
hasO7Pr).
It's possible that the problem had occurred previously and not been
noticed (or reported), though it's also possible this issue had never
occurred before: for the latest freeze (16.1) there were 18 forward
ports spanning the insertion, but of them only 2 were the same batch.
Was missing a logging message in the case where the current and sync'd
head are identical, which seems to occur from time to time but can
only be inferred (by seeing a sync event then nothing happening).
Add a logging warning (because it's a strange situation) in order to
explicitely note the issue.
Also make the sync logging messages more regular for clarity.
And add the delivery information (delivery id and user-agent) to event
log, so it's more possible to report issues to github.
When adding a new project, if no branch matches a base name,
the created bundles won't have a version and it will fail.
A simple fix will be to add a master bundle for all projects.
When an error is linked to another one, we don't expect it to appear on
team and user dashboard. When adding a parent, this will transfer the
responsible from the child to the parent when applicable.
The XML-RPC implementation does not allow for receiving or sending
`None` values (both as query parameters and response).
Since the `write` method of `runbot.bundle` was overriden without
returning a value, an exception is raised when the method is called
through the external API.
This makes the `write` method return the value from its call
to `super()` which should be equal to `True` if all went well.
The auto disable host is mainly usefull when there are a lot of host for
well configured repositories.
If for any reason a repo is corrupted on one host, this host will be
disabled until a manual intervention cleans the repo.
For other cases, where thjere are many repositories with not so many
host, it is most likely that a fetch will fail because of an invalid
repository configuration. Disabling the host in this case is not a good
idea.
With this commit, a settings allows to enable or disable this feature.
At this moment it's difficult to monitor a build cpu and memory usage.
This must be done from outside the container to avoid to kill the cat by
opening the box.
This commit adds a daemonic thread launched by the builder. This thread
will read the memory and cpu stats of the running dockers and report
their stats in a log file into the logs directory of the builds.
The log file format is made to be easily mixed with the build logs and
spot memory leaks or cpu overload during builds.
