Add Py3.9 Compatibility #265
Conversation
* use correct print statements with better str insertion * use better iterators on lists * always decode readlines functions * improve Toolchain.copy function SLIC-3078 http://makerbot.atlassian.net/browse/SLIC-3078
print "string" was replaced with print("string")
replace iteritems with items
SLIC-3078
http://makerbot.atlassian.net/browse/SLIC-3078
* make build repos, get dependencies, and update repos python scripts rather than python2.7 scripts. * toolchain __iter__ method is gone, so iterate over sources. * update glob method in artifactory utils to work with artifactory for py3 * update download and extract arttifacts method to better iterate over a dict.
dudecc
left a comment
dudecc
left a comment
There was a problem hiding this comment.
Not a huge fan of the blanket find and replace across all python scripts if we aren't actually going to test all of the modified scripts. (I did not review any of the changes in the Bundle* directories since that is all dead code anyway but I did look over everything else.)
| # When running this script, be sure that the Environment Variables are on the same usergroup level | ||
| AWS_ACCESS_KEY_ID = os.environ.get('AWS_ACCESS_KEY_ID') | ||
| AWS_SECRET_ACCESS_KEY = os.environ.get('AWS_SECRET_ACCESS_KEY') | ||
| AWS_SESSION_TOKEN = os.environ.get('AWS_SESSION_TOKEN') |
There was a problem hiding this comment.
This doesn't seem to be python 2/3 related...
| "for release (Check for tar.gz source files that shouldn't be " | ||
| "there)\n") | ||
| raw_input("\nThen Press Enter\n")) | ||
| raw_input("\nSecond Safety Check, Press Enter Again")) |
There was a problem hiding this comment.
Why are there extra unmatched parentheses added here? (Also FYI python3 does not have raw_input.)
| Download a single file specified by path to the local filelocation fp | ||
| """ | ||
| print "Downloading file: " + fp | ||
| print("Downloading file: %" % fp) |
There was a problem hiding this comment.
That bare % in the string throws an exception.
| # Pretend mac_pkg_helpers still lives here | ||
| from jenkins_tools import mac_pkg_helpers | ||
| sys.modules['build_tools.mac_pkg_helpers'] = mac_pkg_helpers | ||
| # sys.modules['build_tools.mac_pkg_helpers'] = mac_pkg_helpers |
There was a problem hiding this comment.
Not a huge fan of leaving in commented out code, or comments that no longer accurately describe what the code is doing. (Btw, was this actually a necessary change? I can't find anything that this breaks but I am also not 100% convinced that this won't break anything.)
| cwd=self.dir, | ||
| shell=self.use_shell, | ||
| stderr=stderr) | ||
| stderr=stderr).decode('utf-8') |
There was a problem hiding this comment.
Is this really what we want here? This gets used a lot and it isn't clear to me that it is always used on commands that produce only valid utf-8 output. Decoding as 'ascii' would effectively produce the same behavior as the current python 2 code and never raise a UnicodeDecodeError...
| for a in artifacts: | ||
| print("Looking to download: {}".format(a)) | ||
| rv = a.download(artifact_cache) | ||
| for selector, artifact in artifacts.items(): |
There was a problem hiding this comment.
This function is used in a lot of places, and this change seems to be going from code that will work only when artifacts is a list to code that will only work when artifacts is a dict, but I don't see any changes in this PR that would change what is being passed in. What is going on here?
| # Now any remaining items in the override dict are new and should be added | ||
| for artifact_path in override_dict.itervalues(): | ||
| # for artifact_path in override_dict.itervalues(): | ||
| for artifact_path in override_dict: |
There was a problem hiding this comment.
I think you meant override_dict.values() (also the commented out code is still there)
I ran a bunch of things with both py2 and py3 looking for runtime errors. This seems to work.