Skip to content

Conversation

@paulius-sladkevicius
Copy link
Contributor

@paulius-sladkevicius paulius-sladkevicius commented Nov 9, 2022

The table_columns() slows down modules loading significantly up to 2-3x.
In most cases model._name.startswith(self._model_prefix) returns False, so better to check it first.

@OCA-git-bot
Copy link
Contributor

Hi @legalsylvain,
some modules you are maintaining are being modified, check this out!

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes senses.

note : could you take a look on the both migration in V16 and make a review ? :

  • sql_request_abstract : #669
  • bi_sql_editor : #670

I'll cherry-pick your commit, once merged.

In the meantime, fasttracking trivial patch.

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-683-by-legalsylvain-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 9, 2022
Signed-off-by legalsylvain
@legalsylvain
Copy link
Contributor

Note : pre-commit test is red. you should fix something I guess.

@OCA-git-bot
Copy link
Contributor

@legalsylvain your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-683-by-legalsylvain-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@paulius-sladkevicius
Copy link
Contributor Author

@legalsylvain I think it's pre-commit issue as you can see from the error. My change is minor, shouldn't impact tests.

File "/home/runner/.cache/pre-commit/repoxd7mcpcu/py_env-python3/lib/python3.11/site-packages/wrapt/decorators.py", line 34, in <module>
    from inspect import ismethod, isclass, formatargspec
ImportError: cannot import name 'formatargspec' from 'inspect' (/opt/hostedtoolcache/Python/3.11.0/x64/lib/python3.11/inspect.py)

@legalsylvain
Copy link
Contributor

Any idea on that issue @OCA/community-maintainers ?

@paulius-sladkevicius paulius-sladkevicius force-pushed the 14.0-speed-imp-bi_sql_editor branch from 6138f56 to cb48eaf Compare November 9, 2022 12:31
@paulius-sladkevicius
Copy link
Contributor Author

Any idea on that issue @OCA/community-maintainers ?

@legalsylvain as I'm not aware of this team (at least I can't see it), can you tag the specific person who could now about the pre-commit issue? Thanks.

@simahawk
Copy link
Contributor

The addons repo template has been updated to fix this.
To fix this repo you can:

  1. checkout branch 14.0
  2. run copier -f update
  3. stage changes and commit
  4. open another PR that will be fast tracked when green

Rebase here when done.

The `table_columns()` slows down modules loading significantly up to 2-3x.
In most cases `model._name.startswith(self._model_prefix)` returns `False`, so better to check it first.
@paulius-sladkevicius paulius-sladkevicius force-pushed the 14.0-speed-imp-bi_sql_editor branch from cb48eaf to 4a1209a Compare November 16, 2022 10:19
@paulius-sladkevicius
Copy link
Contributor Author

@legalsylvain we're green.

@legalsylvain
Copy link
Contributor

Thanks ! @paulius-sladkevicius. Could you take a look and make a PR against the migration in V16, I'll merge your patch.

sql_request_abstract : #669
bi_sql_editor : #670

@paulius-sladkevicius
Copy link
Contributor Author

Thanks ! @paulius-sladkevicius. Could you take a look and make a PR against the migration in V16, I'll merge your patch.

sql_request_abstract : #669 bi_sql_editor : #670

Will do

@paulius-sladkevicius
Copy link
Contributor Author

@legalsylvain here it goes legalsylvain#2

@paulius-sladkevicius
Copy link
Contributor Author

@legalsylvain when it is possible to merge it?

@legalsylvain
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-683-by-legalsylvain-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 7c4b7e9 into OCA:14.0 Nov 16, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at a8a2ac5. Thanks a lot for contributing to OCA. ❤️

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants