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

Add jib-uploader action #175

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add jib-uploader action #175

wants to merge 5 commits into from

Conversation

irux
Copy link
Contributor

@irux irux commented Jan 29, 2024

No description provided.

runs:
using: "composite"
steps:
- name: Build Docker image
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- name: Build Docker image
- name: Build & push Docker image

description: "The entrypoint class to be used for the image"
required: true
working-directory:
description: "working directory to run the commands"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: "working directory to run the commands"
description: "Working directory to run the commands"

./gradlew jib \
--info --stacktrace \
--image=${{ inputs.image }} \
-Djib.container.mainClass=${{ inputs.class }} \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really needs to be required?
It think the user can configure this in its own application.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure to be honest, should I remove it ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can definitely be set in the build.gradle file of the specific project as well, so yes please remove it here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we still support it as an optional feature or should we remove it completely? what do you guys think ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have it removed. The user should take of the mainClass 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make it optional as well if there are multiple jib images to be built with different main classes. But I don't have such a use case so I would be fine with removing it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philipp94831 following up this. But how would it work when you have multiple entry points in every module of your project ? do we have such thing atm ? or it is not something it happens quite often ? if not, then I would just remove it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User needs to configure mainClass in gradle file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok then I will remove it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about java-gradle-build-push-jib as the action name, so that it can easier be found when looking for Java Gradle actions?

required: true
class:
description: "The entrypoint class to be used for the image"
required: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this optional. We often times define it in the build.gradle file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See discussion above

@yannick-roeder
Copy link
Member

@irux A corresponding action has already been merged now and I think this PR can be closed: https://github.com/bakdata/ci-templates/blob/main/actions/java-gradle-build-jib/action.yaml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants