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

Only reinit rules when inventory capacity enabled #18306

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cconard96
Copy link
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.
  • This change requires a documentation update.

Description

Fixes #18304
Rules should only be re-initialized when the inventory capacity of an asset definition is enabled, not every time the capacity is bootstrapped. Rules take a long time to initialize even for a single class, and the bootstrap process happens for every request.

Copy link
Contributor

@trasher trasher left a comment

Choose a reason for hiding this comment

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

Tests are broken

@trasher
Copy link
Contributor

trasher commented Nov 15, 2024

I guess the problem is you rely on post_update to add rules. That's correct from the UI point of view; but from tests, capacities are added on add; there is no update.

@cconard96
Copy link
Contributor Author

cconard96 commented Nov 15, 2024

I did have a commit (I removed it after) that added code for post add too, but the tests still fail. I struggled with various combinations of bootstrapping the classes and capacities. When running a single test, it worked. If a second test runs, it fails as if the new asset definition isn't bootstrapped.

@trasher
Copy link
Contributor

trasher commented Nov 15, 2024

I've tried to use post_add; but I got problems on rules initialization. Since it was previously done on asset class bootstrap; the custom asset was entirely created - while it's not using post_add.

No further idea for now.

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.

Custom asset definitions add noticeable request performance hit
2 participants