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

import statement cleanups #63335

Closed
wants to merge 3 commits into from
Closed

import statement cleanups #63335

wants to merge 3 commits into from

Conversation

rhansen
Copy link
Contributor

@rhansen rhansen commented Dec 19, 2022

What does this PR do?

Fixes some minor issues with import statements.

What issues does this PR fix or reference?

None.

Behavior Changes

None.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

The `salt.loader` module is intentially imported late in
`salt.config.call_id_function()` to avoid a circular import error.
However, `salt.loader` is also used in `salt.config.mminion_config()`,
so import it there as well.  This avoids an error if
`mminion_config()` is called before `call_id_function()`, or if the
import statement in `call_id_function()` is removed in the future.
The delayed `import` statements match `import` statements at the top
of the file so they are effectively no-ops.
@rhansen rhansen marked this pull request as ready for review December 19, 2022 06:10
@rhansen rhansen requested a review from a team as a code owner December 19, 2022 06:10
@rhansen rhansen requested review from waynew and removed request for a team December 19, 2022 06:10
@rhansen rhansen mentioned this pull request Dec 19, 2022
3 tasks
import salt.config
# Without this global declaration, the late `import` statement below causes
# `salt` to become a function-local variable.
global salt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not make salt a global.
The import that was in place was working.

If we really wanted to fix this import, we'd have to track down the circular import issue, and most likely, it's not an easy circular import to fix.

Copy link
Contributor Author

@rhansen rhansen Dec 25, 2022

Choose a reason for hiding this comment

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

salt is already a global, just not in this function (without this change) due to the import statement below.

@dwoz
Copy link
Contributor

dwoz commented Dec 11, 2023

Closing this due to inactivity. Anyone should feel free to re-open it if they want to see it through to the end in one release cycle.

@dwoz dwoz closed this Dec 11, 2023
@dwoz dwoz added the Abandoned label Dec 11, 2023
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.

3 participants