-
Notifications
You must be signed in to change notification settings - Fork 0
feat(infra): Add a Dockerfile for Superset with our extra packages in it #171
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe PR introduces Docker container infrastructure for Superset, including a new GitHub Actions workflow to automate Docker image publishing, a Dockerfile configured with Python dependencies and Playwright support, and documentation for the container images directory. Changes
Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
infra/container-images/analytics-data-platform-superset/Dockerfile (1)
18-19: Be aware of significant image size impact from Playwright and Chromium.Installing Playwright with Chromium dependencies adds substantial size to the Docker image. Docker images containing Playwright can be in gigabytes. Ensure this aligns with your deployment and storage requirements. Consider whether both
install-depsandinstall chromiumare necessary together, or if selective browser installation would suffice.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
.github/workflows/docker-superset.yml(1 hunks)infra/container-images/analytics-data-platform-superset/Dockerfile(1 hunks)infra/container-images/readme.md(1 hunks)
🔇 Additional comments (2)
infra/container-images/readme.md (1)
1-3: Good documentation for the container images directory.Clear and concise overview of the directory's purpose.
infra/container-images/analytics-data-platform-superset/Dockerfile (1)
10-16: Verify that the base image and its /app/docker/pip-install.sh script are accessible and compatible.The Dockerfile references a base image
ghcr.io/isisneutronmuon/superset-forkfrom the container registry, but no corresponding public GitHub repository exists to verify its contents. Ensure the base image is available to the build process and that/app/docker/pip-install.shexists within it and supports the--requires-build-essentialargument. Consider documenting the base image's source or building pip-install.sh locally within this Dockerfile if the base image is not reliably available.
Adds a GitHub action to build and optionally publish the image
025e0f4 to
dc15b14
Compare
Don't publish yet until we have run the action once on main.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
Creates a Dockerfile for Superset with our extra packages in it. It is currently based on the fork we maintain at github.com/ISISNeutronMuon/superset-fork.
Adds a GitHub action to build and optionally publish the image.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.