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.

Sign up for GitHub

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

Jump to bottom

Docker: solve polkadot relay folder renaming and other small fixes #1628

Merged
merged 18 commits into from
Nov 30, 2023
Merged

Docker: solve polkadot relay folder renaming and other small fixes #1628

merged 18 commits into from
Nov 30, 2023

Conversation

gpmayorga
Copy link
Contributor

@gpmayorga gpmayorga commented Nov 27, 2023 •
edited
Loading

Includes:

  • Polkadot relays now expect their folder to be named "polkadot" rather than "relay-chain". The new docker container is made aware of this and will rename the folder if needed.
  • Dockerfile build based on Parity "rust-ready" production base image
  • More lean Dockerfile

@gpmayorga gpmayorga requested a review from wischli as a code owner November 27, 2023 10:31
@gpmayorga gpmayorga closed this Nov 27, 2023
@gpmayorga gpmayorga reopened this Nov 27, 2023
@gpmayorga gpmayorga marked this pull request as draft November 27, 2023 10:38
@gpmayorga gpmayorga changed the title DEBUG: activate Docker push to test Docker: solve polkadot relay folder renaming and other small fixes Nov 27, 2023
@gpmayorga gpmayorga added Q1-easy Can be done by primarily duplicating and adapting code. I3-annoyance The code behaves as expected, but "expected" is an issue. I12-ci Issue related to CI github_actions Pull requests that update GitHub Actions code labels Nov 27, 2023
@gpmayorga gpmayorga marked this pull request as ready for review November 29, 2023 09:48
wischli
wischli previously approved these changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! 🤞 switching to Parity as docker base image won't bite us long term

.github/dependabot.yml
@@ -4,4 +4,4 @@ updates:
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "daily"
interval: "monthly"
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀

.github/workflows/build-docker.yml Outdated Show resolved Hide resolved
docker/scripts/entrypoint.sh Outdated Show resolved Hide resolved
@gpmayorga @wischli
Update docker/scripts/entrypoint.sh
c4e247a 
comment of polkadot version

Co-authored-by: William Freudenberger <[email protected]>
@gpmayorga
Copy link
Contributor Author

@wischli the PR is ready for prime time, I opened a couple of discussions for some decisions I want to double-check but this should be a pretty solid Docker strategy at this point

@gpmayorga gpmayorga added the D0-ready Pull request can be merged without special precaution and notification. label Nov 29, 2023
mustermeiszer
mustermeiszer reviewed Nov 30, 2023
View reviewed changes
Copy link
Collaborator

@mustermeiszer mustermeiszer left a comment

Choose a reason for hiding this comment

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

From my limit knowledge this looks really good!

Will approve once somebody with more knowledge cann second my opinion ^^

wischli
wischli approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

Thanks for adhering to my suggestions, looks good to me!

mustermeiszer
mustermeiszer approved these changes Nov 30, 2023
View reviewed changes
Copy link
Collaborator

@mustermeiszer mustermeiszer left a comment

Choose a reason for hiding this comment

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

Based on @wischli review.

@gpmayorga gpmayorga merged commit 36343f5 into main Nov 30, 2023
10 checks passed
@gpmayorga gpmayorga deleted the docker-enhancements branch November 30, 2023 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mustermeiszer mustermeiszer mustermeiszer approved these changes

@wischli wischli wischli approved these changes

@NunoAlexandre NunoAlexandre Awaiting requested review from NunoAlexandre

@lemunozm lemunozm Awaiting requested review from lemunozm lemunozm is a code owner

Assignees
No one assigned
Labels
D0-ready Pull request can be merged without special precaution and notification. github_actions Pull requests that update GitHub Actions code I3-annoyance The code behaves as expected, but "expected" is an issue. I12-ci Issue related to CI Q1-easy Can be done by primarily duplicating and adapting code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gpmayorga @wischli @mustermeiszer