Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STORM-3897 Replace Travis with GitHub Actions #3520

Merged
merged 1 commit into from Mar 8, 2023

Conversation

rzo1
Copy link
Contributor

@rzo1 rzo1 commented Mar 7, 2023

What is the purpose of the change

  • Adds GitHub actions on the basis of the old travis script (and similar to them)
  • Downgrades TestNG as it isn't compatible with Java 8
  • Fixes license files

Note: integration-tests are failing as the related .sh is highly Travis specific and needs some rework. IMHO, we should fix it in a separate Jira. Wdyt?

It superseeds #3519

How was the change tested

  • GitHub Actions

@rzo1 rzo1 changed the title GitHub Action CI for Storm STORM-3897 Replace Travis with GitHub Actions Mar 7, 2023
@rzo1
Copy link
Contributor Author

rzo1 commented Mar 7, 2023

@bipinprasad Feel free to have a look at this PR. It should build wit 8 / 11 for most of the modules. I didn't touch the Integration-Test module atm as it requires some rewriting of the shell script to work with GH. I suggest, that we tackle this part in a separate PR, so we have build feedback, which is crucial (see downgrade of TestNG + license files). WDYT?

@bipinprasad
Copy link
Contributor

Looks good. Thanks. It reverts a security fix (which was incompatible with jdk8). We will have to drop jdk8 and add jdk17 very soon. But till then, this will have to do.

@bipinprasad
Copy link
Contributor

https://issues.apache.org/jira/browse/STORM-3898 for Integration test github action

@rzo1
Copy link
Contributor Author

rzo1 commented Mar 7, 2023

Thanks for the review!

Looks good. Thanks. It reverts a security fix (which was incompatible with jdk8). We will have to drop jdk8 and add jdk17 very soon. But till then, this will have to do.

I agree, that we should drop Java 8 at some point (given that Java 21 will land soon). If you are talking about https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-4065, I don't think, that the test dependency will give a huge attack vector (as we do not ship it) as (in the context of the project) only processes trusted content, no?

@bipinprasad
Copy link
Contributor

Agreed. No problem security-wise, and not an issue at this point.

@bipinprasad bipinprasad merged commit 67472cf into apache:master Mar 8, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants