marcoabreu commented on a change in pull request #18222:
URL: https://github.com/apache/incubator-mxnet/pull/18222#discussion_r419317553
##########
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:
My concern here is that we'd be changing the directory structure in s3 -
which we then have to fix later on.
I'd propose to either introduce the platform identifier immediately or
replace the os parameter with a matching value (replace ubuntu1804 with Ubuntu
1604) as a hotfix. But I'd prefer if we do not change the directory structure
in the underlying system as that'd be something we have to clean up later.
----------------------------------------------------------------
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
