apache / streampipes

Apache StreamPipes - A self-service (Industrial) IoT toolbox to enable non-technical users to connect, analyze and explore IoT data streams.
https://streampipes.apache.org
Apache License 2.0
566 stars 174 forks source link

Execute jobs in pr validation workflow only when required #1102

Open bossenti opened 1 year ago

bossenti commented 1 year ago

Body

With our current PR validation workflow all jobs for checking the PR are run no matter what changes are contained. This results in lot of wasted runs, e.g., running the Python checks when only Java code is commited, and higher run times. As an outcome of this issue, the individual jobs of the PR validation workflow should only be run when they are required by the changes of the underlying PR.

Mentoring

As this ticket is marked as good first issue: @tenthe, @dominikriemer or @bossenti are happy to provide help for getting started. This issue migh be especially suitable for first-time committers having some experiences with GitHub actions.

StreamPipes Committer

I acknowledge that I am a maintainer/committer of the Apache StreamPipes project.

yuktea commented 1 year ago

Hi @bossenti I want to work on this issue.

bossenti commented 1 year ago

welcome to StreamPipes @yuktea 👋🏼

The aim of the ticket is to run a step of our PR validation only when the PR's requires it. Only the E2E tests as well as the Maven RAT plugin should be executed for every run.

Are there any open questions to you?

bossenti commented 1 year ago

The paths attribute of GitHub workflows might be relevant here: https://docs.github.com/de/actions/using-workflows/workflow-syntax-for-github-actions#onpushpull_requestpull_request_targetpathspaths-ignore

yuktea commented 1 year ago

@bossenti: Hi 👋 I opened a draft PR for this. In my PR, I separated the python workflows into a different yaml file and added a path attribute. Looks like the E2E tests and the Maven RAT have a dependency on some of the python workflows. If this dependency is here just so that they execute sequentially, I believe we can remove it in that case.

bossenti commented 1 year ago

great, @yuktea!

I think it makes sense to move the Python stuff to a dedicated workflow file. The dependencies were only introduced to avoid the long running steps when one of the fast ones already failed. So you can safely remove the python-related dependencies.

This is good first iteration which we could already merge. However, I could image to improve this to be even more fine granular and applying similar modifications for he UI and backend as well. But therefore we probably need something like https://github.com/dorny/paths-filter to get the desired behavior.

yuktea commented 1 year ago

Thank you for the review, @bossenti. I'll work on another iteration soon to factor in the modifications you suggested for the UI/backend tests. From my understanding, we'll only run the UI tests when there are UI changes and similarly for the backend.

On a separate note, I wanted to discuss the project you're mentoring for OSPP 2023. I sent you an email about this, but I wanted to confirm if email is your preferred method of communication.

bossenti commented 1 year ago

That sounds great, I'm already looking forward to it. Just be aware that the E2E tests and the RAT plug-in should always run (like mentioned in the description).

Sure, email is fine Unfortunately, I will not have the possibility to look it up until Sunday... but I will read and respond to your message for sure! 🙂

bossenti commented 1 year ago

@yuktea I just replied to your email, hope this reaches you well!