Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC : Generating the analysis files automatically in a cronjob #4707

Open
legalsylvain opened this issue Jan 7, 2025 · 7 comments
Open

RFC : Generating the analysis files automatically in a cronjob #4707

legalsylvain opened this issue Jan 7, 2025 · 7 comments

Comments

@legalsylvain
Copy link
Contributor

legalsylvain commented Jan 7, 2025

Hi. following the proposal of @hbrunn here.

I'm also looking into generating the analysis files automatically in a cronjob so that we always have the latest analysis on github without human intervention.

I propose to talk about that interesting topic here.

rational : (if I understand correctly, correct me if I'm wrong @hbrunn ).
(Exemple : 17.0 -> 18.0)

  • The analysis files are generated at the beginning of a new Odoo version, with the upgrade_analysis module, that is in OCA/server-tools. https://github.com/OCA/server-tools/tree/18.0/upgrade_analysis by any Openupgrade maintainers that is initializing the project
  • Then, the analysis file are correct, but version 17.0 and 18.0 will continue to have patch / changes in odoo core. Some changes requires to update the analysis file. Current Odoo policy say that changes are possible during 3 years. (https://www.odoo.com/documentation/master/administration/supported_versions.html)
  • After few monthes, so, the analysis files are obsolete and should be updated. For the time being, updating analysis is done manually See for exemple @MiquelRForgeFlow PRs : V16 PR V15 PR, etc..
  • This action could be done regularly by a bot.
  • however, if an analysis is updated on a "done" module, openupgrader should manually check if there is extra changes to do.

Proposal

  • Having a github cronjob on each openupgrade version (that is maintained by Odoo SA), to generate regurlarly (Each 3 monthes ?), new analysis and create a PR.
  • in the text of the PR, generate a message, that contains the list of the modules that has been changed AND that are marked as done (or Nothing to do).

Question : if the PR is created in an OCA branch, only maintainers can work on it. (maybe it's not a huge problem...)

what do you think ?

CC : @OCA/openupgrade-maintainers

@hbrunn
Copy link
Member

hbrunn commented Jan 20, 2025

you can now check out the action and the PR this created.

Will PR this to 18.0 when the init PR is merged, but we could also already do the same for older versions, the version specific parts are the two variables at the beginning, the python/postgres version and the sed expressions for choosing a different version of gevent/greenlet.
The cronjob only runs when it exists on the default branch anyways.

As we see there we've new entries now already, I suggest to run this every week. The PR action seems to be smart enough to update an existing open PR, so I don't expect any problems with too many PRs opened.

@rvalyi
Copy link
Member

rvalyi commented Jan 20, 2025

you can now check out the action and the PR this created.

Will PR this to 18.0 when the init PR is merged, but we could also already do the same for older versions, the version specific parts are the two variables at the beginning, the python/postgres version and the sed expressions for choosing a different version of gevent/greenlet. The cronjob only runs when it exists on the default branch anyways.

As we see there we've new entries now already, I suggest to run this every week. The PR action seems to be smart enough to update an existing open PR, so I don't expect any problems with too many PRs opened.

That's great! I may also suggest to do it daily with the master branch. This way we could kind of monitor live the changes been done in each Odoo modules, that could then be used a bit like I did in https://github.com/akretion/odoo-module-diff-analysis to highlight the very commits and their detailed explanation from Odoo that really impact the migration of each module and help people (or even possibly AI in simple cases) writing the migration scripts...

@MiquelRForgeFlow
Copy link
Contributor

  • however, if an analysis is updated on a "done" module, openupgrader should manually check if there is extra changes to do.
  • By "done", you refer "Done", "Done (partial)", "Nothing to do", and anything else that fills the Status column in the docsource table, right?

  • What about open PRs that are migrating a module?
    I think the bot should also send a notification comment in the affected open PR.

  • How the bot will know if there is extra changes to do? Which criteria?
    My criteria proposal should be:
    -- Any noupdate_changes.xml change (in order to avoid deleting a comment or manually change, or delete translations of templates)
    -- Any deleted ir.model.constraint (in order to safely delete in pre-migration).
    -- Any deleted noupdate record (in order to safely delete in post-migration)
    -- If any record has changed from noupdate to update or viceversa (in order to change noupdate in pre-migration).
    -- If a field has changed the type (if changed to html, for example, needs call of convert_field_to_html)
    -- If a field becomes required.
    -- Any other I am missing.

@legalsylvain
Copy link
Contributor Author

you can now check out the action and the PR this created.

That is a very great result ! Thanks !

That's great! I may also suggest to do it daily with the master branch

@rvalyi, It's not possible, as master branch doesn't exist in the OCA.
(Your proposal requires creating a master branch and maintain it (CI, ...) for OCA/openupgrade and OCA/server-tools repos)

The PR action seems to be smart enough to update an existing open PR, so I don't expect any problems with too many PRs opened.

@hbrunn, do you mean that the bot will :

  • Date 1: Create a PR, and add a commit 1
  • Date 2: Update the existing open PR, and add a commit 2
    ?

@hbrunn
Copy link
Member

hbrunn commented Jan 23, 2025

not by opening a new commit, but by force-pushing over it, you can see that in hbrunn#15 where I ran the v18 update multiple times.

BTW I parametrized the action so that we can call it for different versions, as the cron part needs to happen on the main branch.

The other branches could take the same file to have automatic updates for changes to apriori.py.

Last thing unsolved is how to deal with noupdate_changes.xml, as those are edited quite often. I think we could make it a rule to not load this directly, but an edited copy (just as we don't write into upgrade_analysis, but in upgrade_analysis_work). Then we can still see easily in the PR diff if there's something to do there or not.

@legalsylvain
Copy link
Contributor Author

not by opening a new commit, but by force-pushing over it, you can see that in hbrunn#15 where I ran the v18 update multiple times.

If I understand correctly, the bot generates new PR automatically, (that is great, and saves time regarding manual update for the time being), then openupgrade reviewers has to :

  • review the PR
  • eventualy, make changes
  • then merge

The problem I see is for the reviewers. If I review a PR one day, and I come back the next week, and the commit I reviewed disappeared (erased by a new commit), I think it will be harder to review. If commits are accumulated each time, it's maybe easier for reviewers. What do you think ?

Last thing unsolved is how to deal with noupdate_changes.xml, as those are edited quite often. I think we could make it a rule to not load this directly, but an edited copy (just as we don't write into upgrade_analysis, but in upgrade_analysis_work). Then we can still see easily in the PR diff if there's something to do there or not.

I like this proposal ! It is then very easy to meld the two files, to see the differences.

@hbrunn
Copy link
Member

hbrunn commented Jan 24, 2025

updating the existing PR should be doable, will look into this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants