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.

Sign up for GitHub

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

Jump to bottom

[master] Fix config.items return value #66364

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[master] Fix config.items return value #66364

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion salt/modules/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,4 +561,4 @@ def items():

salt '*' config.items
"""
return __opts__
return __opts__.value()
Copy link
Contributor

@lkubb lkubb Apr 15, 2024 •
edited

Choose a reason for hiding this comment

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

I don't think it's always a NamedLoaderContext. See below.

outdated suggestion ``` ```suggestion try: return __opts__.value() except AttributeError: return __opts__ ```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know in what cases it could be not NamedLoaderContext? As far as I know __opts__ is populated with LazyLoader and to be fair I'm not sure if it can push something different there.

Copy link
Contributor

@lkubb lkubb Apr 15, 2024 •
edited

Choose a reason for hiding this comment

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

Tbh now I'm not sure anymore if what I said is correct. I looked up what I remembered when writing the above, but it might have been Salt-SSH-specific only. Sorry for the noise! :)

In general, I think one of the cases where this might happen in a regular module is if the dunder is initialized to a dict during module import in the top-level module code, but that's not the case here (on the contrary).

Edit: So in this module __opts__is specifically initialized to be a NamedLoaderContext, but once I comment out

salt/salt/modules/config.py

Line 40 in aad71fd

__opts__ = __salt_loader__.named_context("__opts__")

it turns into a dict. Someone with more familiarity with the loader will probably be able to shed some light on this.