Re: [Tarantool-patches] [PATCH v1] gitlab-ci: save sources to new S3 location

[Sergey Bronnikov <sergeyb@tarantool.org>]

Hello, Alexander!

Thanks for the patch. See my 5 comments.

On 02.09.2020 14:46, Alexander V. Tikhonov wrote:
> Changed S3 location for sources tarballs. Also added ability to
> create S3 directory for the tarballs if it was not existed.
> ---
>
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/src-s3-new
>
>   .gitlab.mk | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/.gitlab.mk b/.gitlab.mk
> index 0ce978063..708c69082 100644
> --- a/.gitlab.mk
> +++ b/.gitlab.mk
> @@ -108,6 +108,7 @@ GIT_DESCRIBE=$(shell git describe HEAD)
>   MAJOR_VERSION=$(word 1,$(subst ., ,$(GIT_DESCRIBE)))
>   MINOR_VERSION=$(word 2,$(subst ., ,$(GIT_DESCRIBE)))
>   BUCKET="$(MAJOR_VERSION).$(MINOR_VERSION)"
> +S3_BUCKET="s3://tarantool_repo/sources/$(BUCKET)"
1. Wrong parentheses used around BUCKET.

2. You already have variable BUCKET and add a new variable S3_BUCKET.

For my taste it is difficult to understand what for they need when 
looking on the names.
>   
>   deploy_prepare:
>   	[ -d packpack ] || \
> @@ -130,8 +131,13 @@ source: deploy_prepare
>   	TARBALL_COMPRESSOR=gz packpack/packpack tarball
>   
>   source_deploy: source
> -	aws --endpoint-url "${AWS_S3_ENDPOINT_URL}" s3 cp build/*.tar.gz \
> -		"s3://tarantool.${BUCKET}.src/" --acl public-read
> +	( aws --endpoint-url "${AWS_S3_ENDPOINT_URL}" s3 ls "${S3_BUCKET}/" || \
> +		( rm -rf "${BUCKET}" ; mkdir "${BUCKET}" &&

3. Didn't get a point to use 'ls' operation here.

4. Moreover probably it is better to suppress output from listing 
command as it is not used here.

5. In a command chain above you check existence of remote bucket and 
then remove local directory.

Probably you should check existence of local directory too. Otherwise 
'rm' will fail when directory is not exist.

> +			aws --endpoint-url "${AWS_S3_ENDPOINT_URL}"            \
> +				s3 mv "${BUCKET}" "${S3_BUCKET}" --recursive   \
> +				--acl public-read ) ) &&                       \
> +		aws --endpoint-url "${AWS_S3_ENDPOINT_URL}"                    \
> +			s3 cp build/*.tar.gz "${S3_BUCKET}/" --acl public-read
>   
>   # ###################
>   # Performance testing