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.

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

[CONTENT] Refactoring: Absorb tutorial-like references and content from crate-clients-tools #29

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

amotl
Copy link
Member

@amotl amotl commented Feb 21, 2024

About

Previously, a bit of tutorial-like material, mostly in form of references/links, has been slotted into crate-clients-tools as an interim solution. Let's have them play on the main stage now.

Status

This patch is merely a content transfer / refactoring, without adding any style yet. The outcome currently has many walls of links, which is unpleasant. Sure enough, more can be done on behalf of subsequent iterations, in order to provide better appearance and guidance. For this patch, we can be happy enough that the content finally found the right slot.

Preview

Backlog

When integrating this patch, this other one can be merged afterwards, in order to conclude the content migration.

/cc @karynzv, @marijaselakovic, @matkuliak, @hammerhead, @hlcianfagna, @surister, @proddata, @ckurze, @stephanec76

@amotl amotl marked this pull request as ready for review February 21, 2024 11:34
@amotl amotl requested a review from matriv February 21, 2024 11:51
@amotl amotl force-pushed the amo/absorb-tutorial-references branch from a9c8c03 to c524ef9 Compare February 21, 2024 21:21
@amotl amotl changed the title Refactoring: Absorb tutorial-like references from crate-clients-tools Refactoring: Absorb tutorial-like references and content from crate-clients-tools Feb 21, 2024
@amotl amotl changed the title Refactoring: Absorb tutorial-like references and content from crate-clients-tools [CONTENT] Refactoring: Absorb tutorial-like references and content from crate-clients-tools Feb 21, 2024
@amotl amotl force-pushed the amo/absorb-tutorial-references branch from c524ef9 to abc5746 Compare February 21, 2024 22:04
@seut seut removed the request for review from matriv February 22, 2024 07:42
Copy link
Member

@seut seut left a comment

Choose a reason for hiding this comment

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

👍 Added some questions and suggestions. I think the menu structure isn't good as now, see my related comment about integrations vs topics.

Comment on lines +22 to +29
- [Importing Parquet files into CrateDB using Apache Arrow and SQLAlchemy]
- [Guide to efficient data ingestion to CrateDB with pandas]
- [Guide to efficient data ingestion to CrateDB with pandas and Dask]
- [Efficient batch/bulk INSERT operations with pandas, Dask, and SQLAlchemy]
- [pandas code examples]
- [Dask code examples]
- Polars
- [Polars code examples]
Copy link
Member

Choose a reason for hiding this comment

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

Is this the right place for this content? Section is about connect while this content looks more like integrations.
Should we add a pandas or so section to the integrations section instead? This would also highlight it better I think.

Copy link
Member Author

@amotl amotl Feb 22, 2024

Choose a reason for hiding this comment

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

I will see what I can do here while compressing the whole "Getting Started" section into a single page, as suggested. Thanks.

Copy link
Member Author

@amotl amotl Feb 22, 2024

Choose a reason for hiding this comment

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

Corresponding improvements have been implemented on behalf of a separate PR, in order to make reviewing easier, and to separate concerns.

@@ -157,6 +157,7 @@ reference-architectures/index
topic/analysis/index
topic/timeseries/index
topic/ml/index
topic/testing
Copy link
Member

Choose a reason for hiding this comment

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

Can we re-order the entries here so that the reference-architecture is not in the middle of integrations and topics? I'd suggest to move the reference-architecture up before integrations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, we can have a look how this looks like, independently of semantic matters. Thanks.

Copy link
Member Author

@amotl amotl Feb 22, 2024

Choose a reason for hiding this comment

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

Implemented with 2b4b45d on behalf of a separate PR, stacked upon this one.


After executing the second SQL statement, the "nyc_taxi" table will be
populated with data. Depending on your cluster configuration this can take
around 40 minutes.
Copy link
Member

Choose a reason for hiding this comment

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

Can a smaller data-set be used? Waiting 40min for a demo feels too long to me.

Copy link
Member Author

@amotl amotl Feb 22, 2024

Choose a reason for hiding this comment

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

I agree. However, I am not touching the content too much on this iteration, so we need to carry this over into a subsequent round of content editing, which is definitively planned after the current round of structural improvements. Thanks for spotting!

Copy link
Member Author

Choose a reason for hiding this comment

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

Diverted into an issue, thanks again.


# Data Analysis and Visualization

This section of the documentation covers integrations of CrateDB with other
Copy link
Member

Choose a reason for hiding this comment

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

This talks also about integrations, such I wonder if we should merge all this content into the integrations section. Otherwise it feels a bit weird to me, some stuff resides inside the integration section while other not while the content seems to be all about integrations. What is the reasoning for this separation? It's not clear to me as a reader...

Copy link
Member Author

@amotl amotl Feb 22, 2024

Choose a reason for hiding this comment

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

This is right on the spot! I've certainly also identified this as a smell, but continued to bring in the content first. On another iteration here, within the scope of this PR, I will adjust the page layout to reflect the structure of https://cratedb.com/docs/clients/ as the baseline for further discussions. Thanks again!

Copy link
Member Author

Choose a reason for hiding this comment

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

Layout/shape/structure has been improved with d1c46ca and 2b4b45d on behalf of the other PR.

@amotl
Copy link
Member Author

amotl commented Feb 22, 2024

Building upon the comments shared above, @seut suggested a few more details. Thanks.

The first two items will be addressed on behalf of this PR, the second two items are not easy to achieve right now, and need to be postponed into a subsequent iteration.

Previously, a bit of tutorial-like material, mostly in form of
references/links has been slotted into crate-clients-tools as an interim
solution. Let's have them play on the main stage now.
Copy link
Member

@seut seut left a comment

Choose a reason for hiding this comment

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

👍

@amotl amotl merged commit 4643050 into main Feb 23, 2024
3 checks passed
@amotl amotl deleted the amo/absorb-tutorial-references branch February 23, 2024 08:43
@amotl amotl mentioned this pull request Feb 29, 2024
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants