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

Add missing S3 module import #28740

Merged
merged 1 commit into from Nov 10, 2015
Merged

Conversation

MasterNayru
Copy link
Contributor

In the get_url method in fileclient.py, a salt.utils.s3.query method call is made, but this won't work unless the salt.utils.s3 module is imported first. This missing line breaks file.managed resources where source and source_hash attributes use s3:// style URLs.

In the get_url method, a salt.utils.s3.query method call is made, but this won't work unless the salt.utils.s3 module is imported. This missing line breaks file.managed resources where source and source_hash attributes use s3:// URLs.
cachedout pushed a commit that referenced this pull request Nov 10, 2015
@cachedout cachedout merged commit aeafdd5 into saltstack:develop Nov 10, 2015
@cachedout
Copy link
Contributor

Thanks, @MasterNayru

@MasterNayru MasterNayru deleted the s3-urls-fix branch November 10, 2015 19:49
@MasterNayru MasterNayru restored the s3-urls-fix branch November 10, 2015 19:55
@MasterNayru
Copy link
Contributor Author

@cachedout Is there anything I should do to get this line added into the 2015.5 and 2015.8 branches as well? As far as I can tell, this is broken in these two versions of Salt as well.

@cachedout
Copy link
Contributor

@MasterNayru I'm sort of surprised this has been broken as far back as 2015.5 without an issue being filed. Do you have an issue that this is linked to? We can definietely backport it. I'm just curious.

@MasterNayru
Copy link
Contributor Author

@cachedout I'm pretty surprised as well as I don't see how any use of salt.utils.s3.query would work without importing the salt.utils.s3 module in that file first, and I don't see that import present in fileclient.py in the 2014.7, 2015.5 or 2015.8 branches.

As far as existing issues go, I haven't seen any direct reference to this bug from my quick search.

@cachedout
Copy link
Contributor

@MasterNayru K, sounds good. We can just mark this one for backporting, but that's certainly curious. At any rate, thanks for your good detective work here. Much appreciated.

@cachedout cachedout added the bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch label Nov 10, 2015
@MasterNayru MasterNayru deleted the s3-urls-fix branch November 10, 2015 20:47
@rallytime rallytime added ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch. and removed bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch labels Nov 11, 2015
rallytime pushed a commit to rallytime/salt that referenced this pull request Nov 11, 2015
rallytime pushed a commit to rallytime/salt that referenced this pull request Nov 11, 2015
rallytime pushed a commit that referenced this pull request Nov 11, 2015
rallytime pushed a commit that referenced this pull request Nov 11, 2015
rallytime pushed a commit that referenced this pull request Nov 11, 2015
@cachedout
Copy link
Contributor

This is causing a circular import and some serious breakage. I am going to revert this. We need to find a better way to handle this.

bpython version 0.14.2 on top of Python 2.7.10 /usr/bin/python2
>>> import salt.utils
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/home/mp/devel/salt/salt/__init__.py", line 44, in <module>
    from salt.utils import migrations
  File "/home/mp/devel/salt/salt/utils/__init__.py", line 112, in <module>
    import salt.payload
  File "/home/mp/devel/salt/salt/payload.py", line 14, in <module>
    import salt.crypt
  File "/home/mp/devel/salt/salt/crypt.py", line 33, in <module>
    import salt.minion
  File "/home/mp/devel/salt/salt/minion.py", line 60, in <module>
    import salt.client
  File "/home/mp/devel/salt/salt/client/__init__.py", line 38, in <module>
    import salt.utils.event
  File "/home/mp/devel/salt/salt/utils/event.py", line 73, in <module>
    import salt.state
  File "/home/mp/devel/salt/salt/state.py", line 29, in <module>
    import salt.pillar
  File "/home/mp/devel/salt/salt/pillar/__init__.py", line 14, in <module>
    import salt.fileclient
  File "/home/mp/devel/salt/salt/fileclient.py", line 29, in <module>
    import salt.utils.s3
  File "/home/mp/devel/salt/salt/utils/s3.py", line 19, in <module>
    import salt.utils.xmlutil as xml
AttributeError: 'module' object has no attribute 'utils'
>>> 

cachedout pushed a commit to cachedout/salt that referenced this pull request Nov 12, 2015
rallytime pushed a commit that referenced this pull request Nov 12, 2015
@TronPaul
Copy link
Contributor

@cachedout what version of salt were you running for your stacktrace on #28740 (comment)? Curious what version the circular import was on. I'm wondering if it might be caused by the import salt.utils.s3 within fileclient.Client.get_url on develop.

@cachedout
Copy link
Contributor

@TronPaul I don't remember at all. Probably the HEAD of develop at the time, but I can't say for sure.

@TronPaul
Copy link
Contributor

@cachedout I think the weirdness might only exist on branches that have the salt.utils.s3 import in the Client.get_url method. I think I'll try and repro on a few different versions without the Client.get_url import, but with the import at the top of the file and vice versa.

@TronPaul
Copy link
Contributor

@cachedout I did see the circular import in the 2014.7 branch. I'm not seeing it in develop or 2015.5 (I removed the import in Client.get_url and ran the highstate I was having problems with). I think if the import in Client.get_url is removed in both those branches, we'll fix the problem seen in #28883. Possibly adding the salt.utils.s3 import to Client.get_url in 2014.7 might fix it there.

I'll open up a PR for 2015.5 and develop. I'm using features that aren't in 2014.7 so I'm not able to test if moving the import to Client.get_url will work.

@TronPaul TronPaul mentioned this pull request Nov 23, 2015
@cachedout
Copy link
Contributor

@TronPaul OK, I'm going to proceed with your PRs on this one to get things back into working order. We'll need to find another fix for the original motivation behind this PR.

@alangarf
Copy link
Contributor

alangarf commented Dec 1, 2015

Got any further with this?

@TronPaul
Copy link
Contributor

TronPaul commented Dec 1, 2015

@alangarf develop is now working again, and 2015.5 got a backport #29113 as well. I'm not sure what the status of 2014.7 is. My guess is an import of salt.utils.s3 still needs to be added inside the Client.get_url method, though I haven't checked yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants