fix: docker-compose file and superset-core #35235
Conversation
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
betodealmeida
force-pushed
the
fix-docker-compose
branch
from
a37af65 to
932d074
Compare
betodealmeida
force-pushed
the
fix-docker-compose
branch
from
932d074 to
dcab004
Compare
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
I've completed my review and didn't find any issues.
Files scanned
| File Path | Reviewed |
|---|---|
| docker/docker-entrypoint.sh | ✅ |
| docker/docker-bootstrap.sh | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
docker/docker-bootstrap.sh
Outdated
| if [ "$(whoami)" = "root" ] && command -v uv > /dev/null 2>&1; then | ||
| echo "Installing critical dependencies for dev mode..." | ||
| # Install pydantic first as it's needed for imports | ||
| uv pip install --no-cache-dir "pydantic>=2.8.0" |
There was a problem hiding this comment.
This should be in requirements/development.txt already (?)
docker/docker-bootstrap.sh
Outdated
| # Install pydantic first as it's needed for imports | ||
| uv pip install --no-cache-dir "pydantic>=2.8.0" | ||
| echo "Installing base requirements" | ||
| uv pip install -r /app/requirements/base.txt |
There was a problem hiding this comment.
Should we do development.txt here?
There was a problem hiding this comment.
@mistercrunch I think there are folks that use it for production even if not recommended 🤷🏼♂️
docker/docker-bootstrap.sh
Outdated
| echo "Installing base requirements" | ||
| uv pip install -r /app/requirements/base.txt | ||
| echo "Reinstalling superset-core in editable mode" | ||
| uv pip install -e /app/superset-core |
There was a problem hiding this comment.
wondering what this is about (?) I have to admit I don't understand the new structure
There was a problem hiding this comment.
@mistercrunch superset-core is similar to superseet-ui-core. I recommend reading [SIP-177] Proposal for SQL Lab Extensions to understand the structure better.
rusackas
added
the
review:checkpoint
docker/docker-entrypoint.sh
Outdated
| fi | ||
|
|
||
| # Now run the actual bootstrap script | ||
| exec "$@" No newline at end of file |
There was a problem hiding this comment.
@betodealmeida You need a blank line at the end here to fix CI 😉
| exec "$@" | |
| exec "$@" | |
There was a problem hiding this comment.
Nice! thks for the fix @betodealmeida. LGTM, yet would be great if we could avoid the hardcoded pydantic install
betodealmeida
requested review from
geido,
eschutho,
rusackas and
kgabryje
as code owners
|
@betodealmeida It seems there's an error yet: https://github.com/apache/superset/actions/runs/17953994393/job/51060324919 |
I'll fix that in a separate PR. |
michael-s-molina
removed
the
review:checkpoint
SUMMARY
docker compose upstopped working because of the introduction ofsuperset-core.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION