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

It doesn't seem like there was a populated salt dunder within the #32677

Conversation

gladiatr72
Copy link
Contributor

What does this PR do?

On v2015.8.8 .utils.boto() was choking on not finding __salt__['config.option']()
Additionally,.utils.boto._get_profile() was stomping on the region argument

What issues does this PR fix or reference?

This one--maybe others? :)

Previous Behavior

specifying a profile and a region would result in all boto_* calls being dispatched to us-east-1

Tests written?

No

It doesn't seem like there was a populated salt dunder within the salt.utils.boto namespace, so, that.

in .utils.boto._get_profile(), when a profile was specified, the getter
logic wasn't checking for the region argument being set.

salt.utils.boto namespace, so, that.

in .utils.boto._get_profile(), when a profile was specified, the getter
logic wasn't checking for the region argument being set.
@cachedout
Copy link
Contributor

Could you clarify how you determined that this wasn't populated, please? It seems like this was working prior to this point.

@gladiatr72
Copy link
Contributor Author

When trying to track down the disfunction I was experiencing with the boto modules' auth profile functionality, I installed a log directive with the within the profile argument test block. After determining that I had salt.utils logging set to crit (upped it to debug), I started seeing KeyError exceptions.

I added a get_profile = _get_profile to expose the function to the utils loader. Using ipython, I instantiated a _utils__ dict and was seeing the same problem. I grabbed a reference to utils.boto from sys.modules to examine the namespace and found salt.utils.boto.__salt__ to be empty.

Looking at salt.loader.utils(), the only dunder assigned to the pack dict is __context__.


(sidenote)

There is a version of modules.config.option in salt.utils.__init__. Perhaps a different topic, but regardless of the decision wrt to this patch, would it make sense to have the config.* logic encapsulated in a utility module that can either be imported or accessed via the utils dunder? I appreciate the addition of the utils loader for the purpose of easing distribution of custom utility module distribution, but injecting minion modules into the utility name space seems to be a bit upside down.

I look forward to your thoughts.

@ninjada
Copy link

ninjada commented Apr 22, 2016

I also, appear to be experiencing this issue.

@rallytime rallytime added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Apr 25, 2016
@cachedout cachedout merged commit 0ebeb58 into saltstack:develop May 5, 2016
@gladiatr72 gladiatr72 deleted the utils.boto__populate_salt_dunder__fix_region_arg_getting_stomped branch May 7, 2016 02:18
justinta pushed a commit to justinta/salt that referenced this pull request May 12, 2016
rallytime pushed a commit that referenced this pull request May 13, 2016
gitebra pushed a commit to gitebra/salt that referenced this pull request May 16, 2016
* upstream/develop: (60 commits)
  allow top cfg to be YAML for consistency and flexibility (current syntax does not allow blank lines or comments) (saltstack#33189)
  allow user to revert vm to the specific snapshot for vmware (saltstack#33234)
  Support for Advanced Policy Firewall (APF) (saltstack#33134)
  Fix list_values, add test (saltstack#33264)
  Gentoo/OpenRC service management enhancements (saltstack#33260)
  Don't send passwords after shim delimiter is found (saltstack#33170)
  Properly report on invalid gitfs/git_pillar/winrepo repos (saltstack#33244)
  Windows: salt fails to start when running in system account (saltstack#33220)
  add nodegroups ext_pillar (saltstack#33179)
  Fix boto_vpc_test to work with saltstack#32677 (saltstack#33214)
  Expose CherryPy option to write access and error logs (saltstack#33226)
  [2015.8] Merge forward from 2015.5 to 2015.8 (saltstack#33207)
  Don't try to kill a parent proc if we can't
  Add a fetch when compiling git_pillar for masterless minions (saltstack#33204)
  Resolve issue with pkg module on Mint Linux (saltstack#33205)
  cloud.clouds.ec2: cache each named node (saltstack#33164)
  Properly handle failed git commands when redirect_stderr=True (saltstack#33203)
  Add pip installed and removed test (saltstack#33178)
  Don't force use of global ssh_config when git identity file is specified (saltstack#33152)
  update 2015.8.9 release notes (saltstack#33198)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants