Skip to content

Conversation

@qinzhang22
Copy link

@qinzhang22 qinzhang22 commented Oct 30, 2023

Split part2 out of #15

This is literally just converting 'commands' to 'subprocess', updating 'print' syntax, converting 'iteritems' to 'items', removing the use of 'types' and some misc changes.

Signed-off-by: Qin Zhang (张琴) <[email protected]>
Signed-off-by: Qin Zhang (张琴) <[email protected]>
Signed-off-by: Qin Zhang (张琴) <[email protected]>
Signed-off-by: Qin Zhang (张琴) <[email protected]>
Signed-off-by: Qin Zhang (张琴) <[email protected]>
@minglumlu
Copy link
Member

Since we actually have at least two approves (from Andy and Lin), merge it now.

@minglumlu minglumlu merged commit e5df238 into xapi-project:master Nov 1, 2023
bernhardkaindl added a commit that referenced this pull request Nov 24, 2023
…usoutput()

Fix the 1st commit of #17 which replaced commands.getstatusoutput() with
subprocess.getstatusoutput() without taking the change of the status code into account:

Unfortunately, the Python3 developers broke the compatibilty: The return code changes:

python2 -c 'import commands  ; print(  commands.getstatusoutput("false"))'
(256, '')
python3 -c 'import subprocess; print(subprocess.getstatusoutput("false"))'
(1, '')

With commands.getstatusoutput(), you had to use this to get the actual exit code:

status = os.WEXITSTATUS(status)

These calls have to be removed because now they just shift away the error code:

As shown at benjaminp/six#207, the operation is just `status >> 8`

Lucily, the status code is checked to against 0 at most places, so there is no change
for these checks. There is only one location where a bit is checked. Fix this location too.

Also, that commit did not take into account that subprocess.get*output do not exist
in Python2, which goes against the directive by Andrew in PR #16 where he requires
that we keep Python2 working:

The current master branch works neither for Python2, nor Python3 - fix this breakage
in the 2nd commit.

Signed-off-by: Bernhard Kaindl <[email protected]>
bernhardkaindl added a commit that referenced this pull request Nov 24, 2023
…usoutput()

Fix the 1st commit of #17 which replaced commands.getstatusoutput() with
subprocess.getstatusoutput() without taking the change of the status code into account:

Unfortunately, the Python3 developers broke the compatibility: The return code changes:

python2 -c 'import commands  ; print(  commands.getstatusoutput("false"))'
(256, '')
python3 -c 'import subprocess; print(subprocess.getstatusoutput("false"))'
(1, '')

With commands.getstatusoutput(), you had to use this to get the actual exit code:

status = os.WEXITSTATUS(status)

These calls have to be removed because now they just shift away the error code:

As shown at benjaminp/six#207, the operation is just `status >> 8`

Luckily, the status code is checked to against 0 at most places, so there is no change
for these checks. There is only one location where a bit is checked. Fix this location too.

Also, that commit did not take into account that subprocess.get*output do not exist
in Python2, which goes against the directive by Andrew in PR #16 where he requires
that we keep Python2 working:

The current master branch works neither for Python2, nor Python3 - fix this breakage
in the 2nd commit.

Signed-off-by: Bernhard Kaindl <[email protected]>
bernhardkaindl added a commit that referenced this pull request Nov 28, 2023
…usoutput()

Fix the 1st commit of #17 which replaced commands.getstatusoutput() with
subprocess.getstatusoutput() without taking the change of the status code into account:

Unfortunately, the Python3 developers broke the compatibility: The return code changes:

python2 -c 'import commands  ; print(  commands.getstatusoutput("false"))'
(256, '')
python3 -c 'import subprocess; print(subprocess.getstatusoutput("false"))'
(1, '')

With commands.getstatusoutput(), you had to use this to get the actual exit code:

status = os.WEXITSTATUS(status)

These calls have to be removed because now they just shift away the error code:

As shown at benjaminp/six#207, the operation is just `status >> 8`

Luckily, the status code is checked to against 0 at most places, so there is no change
for these checks. There is only one location where a bit is checked. Fix this location too.

Also, that commit did not take into account that subprocess.get*output do not exist
in Python2, which goes against the directive by Andrew in PR #16 where he requires
that we keep Python2 working:

The current master branch works neither for Python2, nor Python3 - fix this breakage
in the 2nd commit.

Signed-off-by: Bernhard Kaindl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants