leezu commented on a change in pull request #18222:
URL: https://github.com/apache/incubator-mxnet/pull/18222#discussion_r419033263



##########
File path: cd/utils/artifact_repository.py
##########
@@ -396,7 +396,7 @@ def get_s3_key_prefix(args: argparse.Namespace, subdir: str 
= '') -> str:
     :param subdir: An optional subdirectory in which to store the files. 
Post-pended to the end of the prefix.
     :return: A string containing the S3 key prefix to be used to uploading and 
downloading files to the artifact repository
     """
-    prefix = "{git_sha}/{libtype}/{os}/{variant}/".format(**vars(args))
+    prefix = "{git_sha}/{libtype}/{variant}/".format(**vars(args))

Review comment:
       @marcoabreu this is broken because there is no reason for different 
stages of the CD pipeline the exact same OS version in their AMI. Perhaps the 
author intended to take the OS used for building the binary into account. But 
that's not done and currently the S3 path depends on the AMI. Thus it's broken 
as GPU and CPU AMIs run different OS versions at this time. Thus deletion of 
that unneeded complexity seems to be the best path forward.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to