* [Tarantool-patches] [PATCH v1] gitlab-ci: save sources to new S3 location
@ 2020-09-02 11:46 Alexander V. Tikhonov
2020-09-11 11:10 ` Sergey Bronnikov
2020-09-15 15:18 ` Kirill Yukhin
0 siblings, 2 replies; 5+ messages in thread
From: Alexander V. Tikhonov @ 2020-09-02 11:46 UTC (permalink / raw)
To: Kirill Yukhin, Alexander Turenko; +Cc: tarantool-patches
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)"
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}" && \
+ 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
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Tarantool-patches] [PATCH v1] gitlab-ci: save sources to new S3 location
2020-09-02 11:46 [Tarantool-patches] [PATCH v1] gitlab-ci: save sources to new S3 location Alexander V. Tikhonov
@ 2020-09-11 11:10 ` Sergey Bronnikov
2020-09-11 12:54 ` Alexander V. Tikhonov
2020-09-15 15:18 ` Kirill Yukhin
1 sibling, 1 reply; 5+ messages in thread
From: Sergey Bronnikov @ 2020-09-11 11:10 UTC (permalink / raw)
To: Alexander V. Tikhonov, Kirill Yukhin, Alexander Turenko; +Cc: tarantool-patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Tarantool-patches] [PATCH v1] gitlab-ci: save sources to new S3 location
2020-09-11 11:10 ` Sergey Bronnikov
@ 2020-09-11 12:54 ` Alexander V. Tikhonov
2020-09-11 13:19 ` Sergey Bronnikov
0 siblings, 1 reply; 5+ messages in thread
From: Alexander V. Tikhonov @ 2020-09-11 12:54 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
Hi Sergey, thanks for the review, pplease check my comments below.
On Fri, Sep 11, 2020 at 02:10:14PM +0300, Sergey Bronnikov wrote:
> 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.
>
That's ok for the current string, due to it depends on Makefile rules.
> 2. You already have variable BUCKET and add a new variable S3_BUCKET.
>
BUCKET is a simple bucket name;
S3_BUCKET renamed to S3_BUCKET_URL to make it more informative.
> 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.
>
AWS S3 tools too weak and 'ls' used there to check the existence of the
directory.
> 4. Moreover probably it is better to suppress output from listing command as
> it is not used here.
>
Here it is really needed to save in output the result.
> 5. In a command chain above you check existence of remote bucket and then
> remove local directory.
>
Unfortunately AWS S3 doesn't have any command to create the directory.
The only way to create it is to copy recursively the empty one. So the
local empty directory is needed for it.
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Tarantool-patches] [PATCH v1] gitlab-ci: save sources to new S3 location
2020-09-11 12:54 ` Alexander V. Tikhonov
@ 2020-09-11 13:19 ` Sergey Bronnikov
0 siblings, 0 replies; 5+ messages in thread
From: Sergey Bronnikov @ 2020-09-11 13:19 UTC (permalink / raw)
To: Alexander V. Tikhonov; +Cc: tarantool-patches
LGTM
On 11.09.2020 15:54, Alexander V. Tikhonov wrote:
> Hi Sergey, thanks for the review, pplease check my comments below.
>
> On Fri, Sep 11, 2020 at 02:10:14PM +0300, Sergey Bronnikov wrote:
>> 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.
>>
> That's ok for the current string, due to it depends on Makefile rules.
>
>> 2. You already have variable BUCKET and add a new variable S3_BUCKET.
>>
> BUCKET is a simple bucket name;
> S3_BUCKET renamed to S3_BUCKET_URL to make it more informative.
>
>> 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.
>>
> AWS S3 tools too weak and 'ls' used there to check the existence of the
> directory.
>
>> 4. Moreover probably it is better to suppress output from listing command as
>> it is not used here.
>>
> Here it is really needed to save in output the result.
>
>> 5. In a command chain above you check existence of remote bucket and then
>> remove local directory.
>>
> Unfortunately AWS S3 doesn't have any command to create the directory.
> The only way to create it is to copy recursively the empty one. So the
> local empty directory is needed for it.
>
>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Tarantool-patches] [PATCH v1] gitlab-ci: save sources to new S3 location
2020-09-02 11:46 [Tarantool-patches] [PATCH v1] gitlab-ci: save sources to new S3 location Alexander V. Tikhonov
2020-09-11 11:10 ` Sergey Bronnikov
@ 2020-09-15 15:18 ` Kirill Yukhin
1 sibling, 0 replies; 5+ messages in thread
From: Kirill Yukhin @ 2020-09-15 15:18 UTC (permalink / raw)
To: Alexander V. Tikhonov; +Cc: tarantool-patches, Alexander Turenko
Hello,
On 02 сен 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
I've checked your patch into 1.10, 2.4, 2.5 and master.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-15 15:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02 11:46 [Tarantool-patches] [PATCH v1] gitlab-ci: save sources to new S3 location Alexander V. Tikhonov
2020-09-11 11:10 ` Sergey Bronnikov
2020-09-11 12:54 ` Alexander V. Tikhonov
2020-09-11 13:19 ` Sergey Bronnikov
2020-09-15 15:18 ` Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox