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

Attempt at reducing the possibility of having import cycles when user configs import rez #1918

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JeanChristopheMorinPerso
Copy link
Member

@JeanChristopheMorinPerso JeanChristopheMorinPerso commented Feb 15, 2025

Fixes #1904

Attempt at reducing the possibility of having import cycles when user configs import rez.

A simple user config that simply imported rez.package_order would fail due to circular imports.

This is a big issue with rez where we already have to do a lot of local imports...

Anyway, this PR is just an experiment. It does remove the likelihood of cycles.

Another approach we could use could be something like https://rez.readthedocs.io/en/stable/configuring_rez.html#delay-load but instead of accepting a path, it could accept a function. That function could be safely called after the config is loaded. Not too sure how it would work or if it would even work.

From my analysis, rez.serialise was the most problematic. It's a core module that gets imported in multiple places. It should very much avoid importing modules like rez.config. In an ideal world, none of our "utilities" module should import rez.config.

@JeanChristopheMorinPerso JeanChristopheMorinPerso force-pushed the reduce_import_sycles_from_config branch 2 times, most recently from 8cbf219 to 9dc285d Compare February 15, 2025 23:15
… configs import rez

Signed-off-by: Jean-Christophe Morin <[email protected]>
@JeanChristopheMorinPerso JeanChristopheMorinPerso force-pushed the reduce_import_sycles_from_config branch from 9dc285d to d1b9a45 Compare February 15, 2025 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken procedure for custom package orderers
1 participant