-
Notifications
You must be signed in to change notification settings - Fork 88
docs(presto-clp): Add instructions for connecting CLP's UI to Presto; Simplify some set up instructions. #1310
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
Merged
Merged
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
2e336cb
latest
828f55c
latest
d38cbe7
latest
83c9016
Apply suggestions from code review
davemarco 2156ce1
latest
da3942e
latest
3bab531
latest
6b4c82a
Apply suggestions from code review
davemarco 832ffb2
latest
f3043fe
latest
d8ad955
Apply suggestions from code review
davemarco f9ec9a6
latest
d495f97
latest
43a39cc
Update docs/src/user-docs/guides-using-presto.md
davemarco e09ef68
latest
51ded2a
Merge branch 'pdocs' of https://github.com/davemarco/clp into pdocs
b61b55d
Update docs/src/user-docs/guides-using-presto.md
davemarco 9d82ad9
Merge branch 'main' into pdocs
davemarco e394803
Merge branch 'main' into pdocs
davemarco File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
(i figure this docs PR should provide better context, although the code for the Presto Docker Compose project was originally added in #1132.)
The mitigation documented above is practical but may be inconvenient for users operating in diverse network environments.
In the long term, we plan to integrate the Presto services into the CLP Package Docker Compose project introduced in #1178. Running
du -h tools/deployment/presto-clpshows the directory is only 132 KB, indicating that the integration would not significantly increase the package size while substantially improving user convenience.@coderabbitai please file an issue to track this long-term integration plan.
Since implementing and reviewing this integration may take time, a short-term solution is needed before the next release. After #1178, the
webuiservice may also encounter connectivity issues with the Presto connector, as host network mode was removed in that update.A likely mitigation is to allow the Presto project to attach to an external Docker network defined in the CLP Package Compose project.
@coderabbitai please create a separate issue to track this short-term fix.
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.
🧩 Analysis chain
🏁 Script executed:
Length of output: 229
@junhaoliao, I've created both issues as requested:
Long-term integration: #1497 - Integrate Presto services into CLP Package Docker Compose project
Short-term fix: #1498 - Allow Presto project to attach to external Docker network for CLP Package Compose
Both issues have been assigned to you and include references to PR #1310 and the relevant context.
🧠 Learnings used