Skip to content

Conversation

@DaanHoogland
Copy link
Contributor

@DaanHoogland DaanHoogland commented Apr 15, 2020

Description

print as method
async keyword use in variable name fixed
urllib openurl from urllib2 to urllib.request

This PR covers partially issue #3195.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

@nvazquez
Copy link
Contributor

nvazquez commented Apr 15, 2020

@DaanHoogland in my tests compile and run works fine, but I'm having this problem with the marvin deploy script:

$  python tools/marvin/marvin/deployDataCenter.py -i ../adv-kvm.cfg
  File "tools/marvin/marvin/deployDataCenter.py", line 77
    print "\n=== Data Center Settings are dumped to %s===" % dc_file_path
                                                         ^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print("\n=== Data Center Settings are dumped to %s===" % dc_file_path)?

@nvazquez
Copy link
Contributor

This will need refactor in many places, maybe considering this library? https://docs.python.org/2/library/2to3.html

@DaanHoogland
Copy link
Contributor Author

yes @nvazquez this only addresses the build. There is another PR out for python3 changes (#3730 by @GabrielBrascher) for the system vm. I don't want to merge, as this is kind of independent. I did hit that deploy script as well, we can put that in a separate PR. or here as well, i just want to move this quickly and smaller chunks move faster ;)

Copy link
Member

@GabrielBrascher GabrielBrascher left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @DaanHoogland. Always looking forward for PRs porting Python2 to Python3 :-)

def xml_for(command):
name = command['name']
async = command['async'] and ' (A)' or ''
asyncmethod = command['async'] and ' (A)' or ''
Copy link
Member

Choose a reason for hiding this comment

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

@DaanHoogland shouldn't the header of this python file be changed from #!/cygdrive/c/Python27 to #!/cygdrive/c/Python3?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry if this comment is redundant, I see that @nvazquez mentioned about changing other pieces as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the cygwin specific python is not a good idea in any case #! /bin/env python would be better.
I think this is being taken into account in ther PRs so maybe i should close, @nvazquez @GabrielBrascher ?

@DaanHoogland
Copy link
Contributor Author

The changes here are in #4479 as well, will probably close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants