[NOTICE] Tests of S3-related PRs are not verified by the Travis configuration of the PR

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[NOTICE] Tests of S3-related PRs are not verified by the Travis configuration of the PR

Nico Kruber
I recently came across the fact, that if I changed anything S3 related, these
tests will only run if proper AWS credentials are set up via the environment
variables ARTIFACTS_AWS_BUCKET, ARTIFACTS_AWS_ACCESS_KEY, and
ARTIFACTS_AWS_SECRET_KEY.

I can set them in my personal Travis CI configuration and they are also set in
Apache Flink's configuration. However, they are not set in the Travis CI runs
of pull requests (most likely so they cannot be leaked). This, however, means
that any PRs related to S3 must be handled specially by the committer who
wants to merge them, i.e. the S3 tests should be run before merging the
change. This could be done manually, or by letting the changes run in a branch
of the committer with proper AWS credentials being set up at Travis CI.
Either way, the committer should be aware and hence I propose to extent the PR
template in [1] accordingly.


Nico

[1] https://github.com/apache/flink/pull/4952

signature.asc (201 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [NOTICE] Tests of S3-related PRs are not verified by the Travis configuration of the PR

Till Rohrmann
Thanks for raising this point Nico. I think it is a very good idea to
extend the PR template to include this notice. Thus, +1 from my side.

Cheers,
Till

On Mon, Nov 6, 2017 at 12:18 PM, Nico Kruber <[hidden email]> wrote:

> I recently came across the fact, that if I changed anything S3 related,
> these
> tests will only run if proper AWS credentials are set up via the
> environment
> variables ARTIFACTS_AWS_BUCKET, ARTIFACTS_AWS_ACCESS_KEY, and
> ARTIFACTS_AWS_SECRET_KEY.
>
> I can set them in my personal Travis CI configuration and they are also
> set in
> Apache Flink's configuration. However, they are not set in the Travis CI
> runs
> of pull requests (most likely so they cannot be leaked). This, however,
> means
> that any PRs related to S3 must be handled specially by the committer who
> wants to merge them, i.e. the S3 tests should be run before merging the
> change. This could be done manually, or by letting the changes run in a
> branch
> of the committer with proper AWS credentials being set up at Travis CI.
> Either way, the committer should be aware and hence I propose to extent
> the PR
> template in [1] accordingly.
>
>
> Nico
>
> [1] https://github.com/apache/flink/pull/4952