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



##########
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:
       The existing setup does not build binaries for different OS and 
different architectures. Even when building binaries for other platforms, there 
is no reason for the build platform to be the same as the target platform. 
   
   The current design is not designed correctly and just looks at the build 
platform, ie `cat /etc/os-release`. It needs to be fixed to support the 
use-cases you mention, though we don't support them at this point.
   
   As it's broken by design, I suggest to remove broken functionality. Though I 
agree deleting `{os}` does not amount to removing the broken functionality 
(also `probe_operating_system` etc would need to be removed)




----------------------------------------------------------------
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