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

Make raw imports from _utils work #46841

Closed

Conversation

mephi42
Copy link
Contributor

@mephi42 mephi42 commented Apr 3, 2018

This is a quick hack to make it work with salt-ssh.
It would be great if anyone could outline the proper solution, since I got lost in module loading logic.

What issues does this PR fix or reference?

#32500

Previous Behavior

Importing _utils module resulted in ImportError.

New Behavior

Importing _utils module works as described in https://docs.saltstack.com/en/latest/topics/utils/index.html

Tests written?

No

Commits signed with GPG?

Yes

@mephi42 mephi42 force-pushed the make-raw-imports-from-utils-work branch from 357eddb to 1c26f09 Compare April 4, 2018 09:28
@mephi42 mephi42 requested a review from a team as a code owner April 4, 2018 09:28
Copy link
Contributor

@terminalmage terminalmage left a comment

Choose a reason for hiding this comment

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

This would not be a good idea, it would add utils functions to both __salt__ and __states__.

Can you show an example of what you're trying to do, what you expect to happen, and what is happening instead? Your most recent comment in the linked issue doesn't really offer much information.

@rallytime rallytime added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Apr 4, 2018
@mephi42
Copy link
Contributor Author

mephi42 commented Apr 4, 2018

I'm trying to make the following example from the doc work:

_utils/mymodule.py:

class Foo(object):
    def bar(self):
        return 'baz'

_modules/awesome.py:

import mymodule
def observe_the_awesomeness():
    foo = mymodule.Foo()
    return foo.bar()

In v2017.7.3 it fails with an ImportError.

@terminalmage
Copy link
Contributor

OK, I'm not familiar with this functionality, I will investigate.

@mephi42
Copy link
Contributor Author

mephi42 commented Apr 6, 2018

I see now what you meant in your comment:

[ERROR   ] Module/package collision: '<redacted>/lib/python2.7/site-packages/salt/utils/docker' and '<redacted>/lib/python2.7/site-packages/salt/states/docker.py'
[ERROR   ] Module/package collision: '<redacted>/lib/python2.7/site-packages/salt/utils/pkg' and '<redacted>/lib/python2.7/site-packages/salt/states/pkg.py'

I did not use those states when I tested the patch, so it looked okay.

@cachedout
Copy link
Contributor

We definitely don't want to merge this. I'm going to close this but we can discuss a path forward in #32500.

@cachedout cachedout closed this Apr 12, 2018
@mephi42
Copy link
Contributor Author

mephi42 commented Apr 18, 2018

No problem, let's discuss the possible alternatives there.

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.

4 participants