-
Notifications
You must be signed in to change notification settings - Fork 372
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
Replace module imp with importlib #1738
Merged
Merged
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
f696540
Release 2.2.37 into the master
vrdmr bce7de9
Release 2.2.38 into master
vrdmr 5d3757a
Release 2.2.40 (#1518)
pgombar 0f1f1cf
Version update for release 2.2.41
vrdmr 94e873f
Merging release 2.2.41 into master
vrdmr bc4b721
Fixing the LaunchCommandTestCase
vrdmr a0bb361
Merge pull request #1585 from vrdmr/master
vrdmr aaea41a
Moved code to check if extension processing allowed to a func and run…
larohra efabc2b
Added a test case to test extension processing allowed
larohra 168fc03
Added a test to check the status of last etag
larohra 54d393a
Modified tests to be stateless
larohra e111cbf
Removed dead dependency
larohra 378eb03
Renamed the last_etag to last_processed_etag
larohra f2674dc
Reverted back to last_etag
larohra 4a5db51
Merge pull request #1602 from larohra/PollForArtifactBlob
larohra f8a718b
Updated version to 2.2.42, added new zip and removed old zips
77d4ac1
Merge pull request #1603 from larohra/PollForArtifactBlob
larohra 62b7b01
Merge branch 'develop' into hotfix-2.2.42
larohra 09060e1
Revert "Merge branch 'develop' into hotfix-2.2.42"
2b5ba48
Merge pull request #1611 from Azure/hotfix-2.2.42
larohra d41a335
Bringing Master to Current Levels (#1721)
vrdmr 3487f3e
Merge pull request #1722 from Azure/release-2.2.44
vrdmr dcdc331
Merge pull request #1727 from Azure/release-2.2.45
pgombar 1ffad2b
Replace module imp with importlib
johanburati c23436e
Merge branch 'develop' into issues/1473
pgombar dcbc96f
keep imp for py2
pgombar 7527afa
Merge branch 'develop' into issues/1473
pgombar 91d0eee
Merge branch 'develop' into issues/1473
pgombar 8461e54
Merge branch 'develop' into issues/1473
pgombar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does importlib do anything or potentially going to do anything here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
importlib
is the preferred module sinceimp
is deprecated. We define it here, the same way likeimp
was defined here so far. Does that answer your question?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, i understand that imp is deprecated for python3, and importlib is the succeeder. But i don't see any usage of importlib in this file, and the code snippet where was using 'imp' is never supposed to be executed with python3, so i think it would be the same if we just remove the else part. Please correct me if i'm wrong, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rainer37 I see your point now. You are right, since we have:
imp
would never be executed with python3.However, the imports will always be evaluated, which means that today, in an image that runs the agent with Python3, we would see this:
The purpose of this change is to get rid of this warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pgombar thanks for the info! but sorry i think
is enough to silence the DeprecationWarning for os with /usr/bin/env python as python3* since python3 wouldn't take this branch, and python3 is not going to do any module load using 'importlib'. S adding
does not seem to help in any cases but only import a module that is not used. Any further explanation would be much appreciated, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking only at this file's contents, you are right and there is no need for
importlib
since it is not called anywhere in this copy ofbin/waagent
. However, there are certain implicit dependencies betweenbin/waagent
and VM extensions. In case the extensions need to load modules from the agent, and are using Python3, we are enabling them to do so withimportlib
.