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



##########
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:
       We can hardcode some value for `{os}` instead of deleting it, sure. 
Though this is a internal `ARTIFACT_REPOSITORY_BUCKET` s3 bucket, which is 
private and arguably should use object expiration as access to old objects is 
not needed (later step of the CD publishes the proper build artifacts to a 
public bucket). Thus I  don't think we have to make any guarantees about a 
consistent structure over time.
   
   I have some more concerns about the structure of the CD, but I'll open a 
separate issue to discuss them.




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to