Skip to content

Avoid running Maven in PT CI jobs#14931

Merged
hashhar merged 1 commit intotrinodb:masterfrom
MiguelWeezardo:avoid_maven_in_pt_ci
Nov 7, 2022
Merged

Avoid running Maven in PT CI jobs#14931
hashhar merged 1 commit intotrinodb:masterfrom
MiguelWeezardo:avoid_maven_in_pt_ci

Conversation

@MiguelWeezardo
Copy link
Copy Markdown
Member

@MiguelWeezardo MiguelWeezardo commented Nov 7, 2022

Description

ptl executes run-launcher which uses Maven to find current Trino version.
This has the downside of trying to download Maven plugins, and fails if Maven Central is down.

Example: https://github.com/trinodb/trino/actions/runs/3409303066/jobs/5671117481

These scripts should be run by developers, not by CI where only one Trino is available.

Non-technical explanation

Avoid Maven in product tests to protect from Maven Central outages.

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

`ptl` executes `run-launcher` which uses Maven to find current Trino version.
This has the downside of trying to download all Maven plugin POMs, and
fails if Maven Central is down.
These scripts should be run by developers, not by CI where only one Trino
is available.
@hashhar hashhar added the no-release-notes This pull request does not require release notes entry label Nov 7, 2022
@hashhar hashhar merged commit 2c9c4df into trinodb:master Nov 7, 2022
@github-actions github-actions bot added this to the 403 milestone Nov 7, 2022
@MiguelWeezardo MiguelWeezardo deleted the avoid_maven_in_pt_ci branch November 8, 2022 09:50
@findepi
Copy link
Copy Markdown
Member

findepi commented Nov 17, 2022

These scripts should be run by developers, not by CI where only one Trino is available.

The CI should ensure these scripts do still work, since people depend on them.

Can we fix the Maven Central problem without removing coverage?

@nineinchnick
Copy link
Copy Markdown
Member

We still call it in the build-pt job

@hashhar
Copy link
Copy Markdown
Member

hashhar commented Nov 18, 2022

We still call it in the build-pt job

I don't think so. This was the only usage.

@nineinchnick
Copy link
Copy Markdown
Member

It runs the .github/bin/build-pt-matrix-from-impacted-connectors.py script:
https://github.com/trinodb/trino/blob/master/.github/workflows/ci.yml#L879

which in turn uses the PTL launcher:
https://github.com/trinodb/trino/blob/master/.github/bin/build-pt-matrix-from-impacted-connectors.py#L64

PTL is used to describe suites to get the list of features configured in their test environments. This only happens in PRs. On master, when the list of impacted modules is empty, this script doesn't call PTL and just converts the matrix from YAML to JSON.

@hashhar
Copy link
Copy Markdown
Member

hashhar commented Nov 18, 2022

Thanks for the link, I was looking in ci.yml only.

@MiguelWeezardo
Copy link
Copy Markdown
Member Author

It runs the .github/bin/build-pt-matrix-from-impacted-connectors.py script: https://github.com/trinodb/trino/blob/master/.github/workflows/ci.yml#L879

which in turn uses the PTL launcher: https://github.com/trinodb/trino/blob/master/.github/bin/build-pt-matrix-from-impacted-connectors.py#L64

PTL is used to describe suites to get the list of features configured in their test environments. This only happens in PRs. On master, when the list of impacted modules is empty, this script doesn't call PTL and just converts the matrix from YAML to JSON.

This means our CI pipeline will still fail when Maven Central is down, so this PR didn't achieve much.
Perhaps instead of trying to avoid the dependency on Maven Central we should add more public repos, and hope they don't all go down at once?

@nineinchnick
Copy link
Copy Markdown
Member

This means our CI pipeline will still fail when Maven Central is down, so this PR didn't achieve much.

Not necessarily. build-pt calls Maven but it also restores the Maven cache so there's a high chance it won't actually try to fetch that testng 5.10 jars.

The jobs that actually execute product tests use artifacts prepared by build-pt and don't need to call Maven at all.

@hashhar
Copy link
Copy Markdown
Member

hashhar commented Nov 18, 2022

Re: adding mirrors it was something which came up in recent discussion - if we still notice issues with Maven (with the cache solution) we should consider adding Google's mirror and maybe if GitHub has a mirror (it would be super-fast) to settings.xml on CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

4 participants