Replace module imp with importlib #1738

johanburati · 2019-12-18T02:37:37Z

Description

Issue #1473

https://www.python-future.org/_modules/imp.html

In most cases it is preferred you consider using the importlib module's functionality over this module.

PR information

The title of the PR is clear and informative.
There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
If applicable, the PR references the bug/issue that it fixes in the description.
New Unit tests were added for the changes made and Travis.CI is passing.

Quality of Code and Contribution Guidelines

I have read the contribution guidelines.

This change is

Merge pull request #1492 from Azure/release-2.2.37

Merge pull request #1493 from Azure/release-2.2.38

* Use 1804-style deprovisioning for all versions >= 18.04 (#1483) 18.04 changed the method of managing resolv.conf vs. 16.04; the new method involves letting systemd-networkd manage that file. A previous fix corrected waagent behavior for 18.04 but did not do so for subsequent releases. Now that Ubuntu's direction is clear, this commit makes the correction in all versions of Ubuntu from 18.04 onwards. Other distros are unaffected by this bug and by this fix. * Updating Travis settings * Added description for Extension Error Codes (#1482) * Added description for Extension Error Codes * Correcting the test - to send the correct CRP Extension Error code * Adding docstring to describe ExtensionErrorCodes * Make launch_command resilient to cgroups failures (#1484) * WireServer Certificates parser gracefully exits if format of package is not Pkcs7BlobWithPfxContents (#1474) * Certificates parser gracefully exits if format of package is not Pkcs7BlobWithPfxContents * Add missing files * change message to warning * Removing the requirement of passing cipher-name. Default is more secure (#1481) * Fix PID tracking for cgroups (#1489) * add fixes for cgroup setup and PID tracking * cleanup code comments * disable cgroups test if not supported by environment * remove wrapper cgroup tracking and other nits * fix whitespace * nit fixes; addressing CR comments * Release 2.2.39 (#1494) * Fix header * Update agent version to 2.2.40

Merging release 2.2.41 into master

…ning ext handler only if allowed

Poll for artifact blob

Updated version to 2.2.42, added new zip and removed old zips

This reverts commit 62b7b01, reversing changes made to 77d4ac1.

Merging Hotfix 2.2.42 into master

Merging Release 2.2.44 into Master

Merge release 2.2.45 into master

codecov · 2019-12-18T02:39:26Z

Codecov Report

Merging #1738 into develop will increase coverage by 0.33%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #1738      +/-   ##
===========================================
+ Coverage    69.12%   69.45%   +0.33%     
===========================================
  Files           82       82              
  Lines        11511    11469      -42     
  Branches      1619     1619              
===========================================
+ Hits          7957     7966       +9     
+ Misses        3217     3166      -51     
  Partials       337      337

Impacted Files	Coverage Δ
azurelinuxagent/common/event.py	`86.19% <ø> (ø)`
azurelinuxagent/common/protocol/restapi.py	`99.23% <ø> (ø)`
azurelinuxagent/common/utils/shellutil.py	`66.26% <ø> (ø)`
azurelinuxagent/common/protocol/wire.py	`72.10% <100.00%> (ø)`
azurelinuxagent/ga/exthandlers.py	`88.49% <100.00%> (ø)`
azurelinuxagent/ga/update.py	`88.51% <100.00%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62227d6...8461e54. Read the comment docs.

rainer37 · 2020-04-23T22:19:36Z

bin/waagent

+if sys.version_info[0] == 2:
+    import imp
+else:
+    import importlib


does importlib do anything or potentially going to do anything here?

importlib is the preferred module since imp is deprecated. We define it here, the same way like imp was defined here so far. Does that answer your question?

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!

@rainer37 I see your point now. You are right, since we have:

if sys.version_info[0] == 3: raise ImportError("waagent2.0 doesn't support python3")

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:

root@ubuntu2004:~# waagent --version /usr/sbin/waagent:27: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses import imp WALinuxAgent-2.2.46 running on ubuntu 20.04 Python: 3.8.2 Goal state agent: 2.2.46

The purpose of this change is to get rid of this warning.

@pgombar thanks for the info! but sorry i think

if sys.version_info[0] == 2: import imp

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

else: import importlib

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!

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 of bin/waagent. However, there are certain implicit dependencies between bin/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 with importlib.

larohra

LGTM

pgombar · 2020-04-24T17:17:43Z

@johanburati thanks for your contribution!

vrdmr and others added 24 commits March 26, 2019 13:32

Release 2.2.37 into the master

f696540

Merge pull request #1492 from Azure/release-2.2.37

Release 2.2.38 into master

bce7de9

Merge pull request #1493 from Azure/release-2.2.38

Version update for release 2.2.41

0f1f1cf

Merging release 2.2.41 into master

94e873f

Fixing the LaunchCommandTestCase

bc4b721

Merge pull request #1585 from vrdmr/master

a0bb361

Merging release 2.2.41 into master

Moved code to check if extension processing allowed to a func and run…

aaea41a

…ning ext handler only if allowed

Added a test case to test extension processing allowed

efabc2b

Added a test to check the status of last etag

168fc03

Modified tests to be stateless

54d393a

Removed dead dependency

e111cbf

Renamed the last_etag to last_processed_etag

378eb03

Reverted back to last_etag

f2674dc

Merge pull request #1602 from larohra/PollForArtifactBlob

4a5db51

Poll for artifact blob

Updated version to 2.2.42, added new zip and removed old zips

f8a718b

Merge pull request #1603 from larohra/PollForArtifactBlob

77d4ac1

Updated version to 2.2.42, added new zip and removed old zips

Merge branch 'develop' into hotfix-2.2.42

62b7b01

Revert "Merge branch 'develop' into hotfix-2.2.42"

09060e1

This reverts commit 62b7b01, reversing changes made to 77d4ac1.

Merge pull request #1611 from Azure/hotfix-2.2.42

2b5ba48

Merging Hotfix 2.2.42 into master

Bringing Master to Current Levels (#1721)

d41a335

Merge pull request #1722 from Azure/release-2.2.44

3487f3e

Merging Release 2.2.44 into Master

Merge pull request #1727 from Azure/release-2.2.45

dcdc331

Merge release 2.2.45 into master

Replace module imp with importlib

1ffad2b

johanburati requested review from larohra, narrieta, pgombar and vrdmr as code owners December 18, 2019 02:37

pgombar self-assigned this Jan 7, 2020

vrdmr removed their request for review April 23, 2020 00:05

Merge branch 'develop' into issues/1473

c23436e

pgombar requested a review from ZhidongPeng as a code owner April 23, 2020 18:25

keep imp for py2

dcbc96f

rainer37 reviewed Apr 23, 2020

View reviewed changes

pgombar added 2 commits April 23, 2020 16:35

Merge branch 'develop' into issues/1473

7527afa

Merge branch 'develop' into issues/1473

91d0eee

narrieta approved these changes Apr 24, 2020

View reviewed changes

Merge branch 'develop' into issues/1473

8461e54

narrieta approved these changes Apr 24, 2020

View reviewed changes

larohra approved these changes Apr 24, 2020

View reviewed changes

pgombar merged commit 11d0881 into Azure:develop Apr 24, 2020

pgombar mentioned this pull request Apr 24, 2020

[BUG] Warning: the imp module is deprecated in favour of importlib #1473

Closed

narrieta mentioned this pull request Aug 18, 2021

DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses #2326

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace module imp with importlib #1738

Replace module imp with importlib #1738

johanburati commented Dec 18, 2019 •

edited by vrdmr

Loading

codecov bot commented Dec 18, 2019 •

edited

Loading

rainer37 Apr 23, 2020

pgombar Apr 24, 2020

rainer37 Apr 24, 2020

pgombar Apr 24, 2020

rainer37 Apr 24, 2020

pgombar Apr 24, 2020

larohra left a comment

pgombar commented Apr 24, 2020

Replace module imp with importlib #1738

Replace module imp with importlib #1738

Conversation

johanburati commented Dec 18, 2019 • edited by vrdmr Loading

Description

PR information

Quality of Code and Contribution Guidelines

codecov bot commented Dec 18, 2019 • edited Loading

Codecov Report

rainer37 Apr 23, 2020

Choose a reason for hiding this comment

pgombar Apr 24, 2020

Choose a reason for hiding this comment

rainer37 Apr 24, 2020

Choose a reason for hiding this comment

pgombar Apr 24, 2020

Choose a reason for hiding this comment

rainer37 Apr 24, 2020

Choose a reason for hiding this comment

pgombar Apr 24, 2020

Choose a reason for hiding this comment

larohra left a comment

Choose a reason for hiding this comment

pgombar commented Apr 24, 2020

johanburati commented Dec 18, 2019 •

edited by vrdmr

Loading

codecov bot commented Dec 18, 2019 •

edited

Loading