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

Adding capability to deploy using Docker. #1525

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Harshraj9812
Copy link

@Harshraj9812 Harshraj9812 commented Mar 24, 2024

This pull request...

  • Fixes a bug
  • Introduces a new feature
  • Improves an existing feature
  • Boosts code quality or performance

Description

These changes are to make this awesome bot docker deployment ready for Ease of Deployment.

Things added -

  • Added Github Action to create a Docker Build by providing the Release version
  • Added Dockerfile which will fetch the provided release files from the repo and create a Docker Image
  • Added Sample Docker-Compose file for the user who will be going to deploy this bot with docker.

Config Changes needed

  • Please add Your Docker Hub Secrets on GitHub Secrets with these key values (These are used in the GitHub Action file)
DOCKER_USERNAME: ${{ secrets.DOCKER_USERNAME }}
DOCKER_PASSWORD: ${{ secrets.DOCKER_PASSWORD }}

Purpose

These changes will add a new capability to this bot to be deployed with Docker, Which will ease the Deployment process without any dependency on OS.

Relevant Issue(s)

No fix for any ongoing issue, This is for additional deployment improvement.

Dockerfile Outdated Show resolved Hide resolved
Comment on lines +15 to +16
RUN curl -LJO "https://github.com/jagrosh/MusicBot/releases/download/$MUSICBOT_VERSION/JMusicBot-$MUSICBOT_VERSION.jar" \
&& mv "JMusicBot-$MUSICBOT_VERSION.jar" JMusicBot.jar
Copy link

Choose a reason for hiding this comment

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

It would be preferred to use the build artifacts from the previous job instead of fetching the release file like this. The files attached on a release are not necessarily the same as builds from source code.

Tip: with curl the mv step is also completely unnecessary since you can also save the file as a specified filename

Suggested change
RUN curl -LJO "https://github.com/jagrosh/MusicBot/releases/download/$MUSICBOT_VERSION/JMusicBot-$MUSICBOT_VERSION.jar" \
&& mv "JMusicBot-$MUSICBOT_VERSION.jar" JMusicBot.jar
RUN curl -o JMusicBot.jar -LJ "https://github.com/jagrosh/MusicBot/releases/download/$MUSICBOT_VERSION/JMusicBot-$MUSICBOT_VERSION.jar"

Copy link
Author

Choose a reason for hiding this comment

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

Hi @jogerj

Can you please help me with the change required to get the build form Artificats, I found this way more easier and understandable.

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

Successfully merging this pull request may close these issues.

3 participants