Skip to content

[DOCS] : Fast Follow Connect Fixes #7755

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
Merged
merged 8 commits into from
Aug 12, 2025
Merged

[DOCS] : Fast Follow Connect Fixes #7755

merged 8 commits into from
Aug 12, 2025

Conversation

royendo
Copy link
Contributor

@royendo royendo commented Aug 8, 2025

INSERT DESCRIPTION HERE

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@royendo royendo requested a review from ericokuma August 8, 2025 16:34
@royendo
Copy link
Contributor Author

royendo commented Aug 11, 2025

readd changes to mysql https://github.com/rilldata/rill/pull/7674/files#diff-14951bd1a9d92b9ad970ae83d7166b81324bf99cd82baad770dcaddcebaf6f56

ericpgreen2
ericpgreen2 reviewed Aug 11, 2025
View reviewed changes
Copy link
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

I'll let Eric O do the full review, but leaving a few comments of things I spotted quickly

docs/docs/connect/olap/clickhouse.md Outdated
Comment on lines 26 to 28
### Connection Parameters

<img src='/img/connect/connector/byob-clickhouse.png' class='rounded-gif' />
Copy link
Contributor

@ericpgreen2 ericpgreen2 Aug 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Generally, I find it hard to follow when a section begins with an image. I'd expect a section to start with some introduction, which can help put the image into context.

There are a few cases of this pattern on this page. It's additionally hard when the image nearly takes up the full height of my screen, so once I reach the first text, I can no longer see the title of the section I'm in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed the ordering

trying 75% size?

or do we prefer it in the Rill Dev UI? like https://docs.rilldata.com/connect/olap/druid#configuring-rill-developer-with-druid

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2025-08-11 at 17 05 26

Copy link
Contributor

Choose a reason for hiding this comment

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

That's way better!

@ericokuma
Copy link
Contributor

ericokuma commented Aug 11, 2025 •
edited by royendo
Loading

Feedback

Connect page
Screenshot 2025-08-11 at 2 42 11 PM

  • what are the caveats?
  • Also some grammar improvements: "Full Rill functionality with some caveats depending on which embedded engine you select
Screenshot 2025-08-11 at 2 44 16 PM
  • Cyrus built a UI for this. To create a managed ClickHouse connector (i.e. Rill-managed ClickHouse), they'll need to select "Rill-managed Clickhouse" under connector type. Should we display a screenshot of this?
Screenshot 2025-08-11 at 2 53 40 PM Screenshot 2025-08-11 at 2 46 20 PM
  • Update to: "For large-scale datasets...
  • What are they limitations on modeling for BYO-OLAP?

@ericokuma
Copy link
Contributor

Data Source page
Screenshot 2025-08-11 at 2 57 20 PM
When I click on this link, I see this:
Screenshot 2025-08-11 at 2 58 08 PM
I would expect "Rill Managed ClickHouse" to be anchored to the top of the page. Also, we should rename "managed CH" in this context to "Rill-managed ClickHouse"

Screenshot 2025-08-11 at 3 00 06 PM it's not immediately clear that the features in development are that are causing CH limitations.

@royendo
Copy link
Contributor Author

royendo commented Aug 11, 2025 •
edited
Loading

  • what are the caveats?
  • What are they limitations on modeling for BYO-OLAP?

Adding Caveats header in connect/data-sources#olap-caveats

@royendo
Copy link
Contributor Author

royendo commented Aug 11, 2025 •
edited
Loading

When I click on this link, I see this:

i get this behavior on our actual docs, can you confirm this is the docs branch?

it's not immediately clear that the features in development are that are causing CH limitations.

That's a good comment, i didn't know either till i pinged in Slack. Not sure how deep we should go into this.. or maybe remove altogether?

EDIT: on second thought, removing the CH limitations as we removed Feature flag in 69.

@royendo royendo merged commit 7821f9a into main Aug 12, 2025
7 checks passed
@royendo royendo deleted the docs-fast-follow-fixes branch August 12, 2025 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericpgreen2 ericpgreen2 ericpgreen2 left review comments

@ericokuma ericokuma ericokuma approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@royendo @ericokuma @ericpgreen2