Improve documentation to clarify purpose of the driver #365

amotl · 2023-03-22T16:33:22Z

Following up on GH-364, this patch aims to:

Clarify the purpose of this "legacy" driver (f615472)
- Describe why it is still needed in a few cases.
- Also add the recommendation to use the official driver in other cases.
Make "implementation notes" more prominent (b64ff63)
In order to better educate users about "what's inside", this refactors a section of the documentation from the appendix to the front page.
Add example about Apache Kafka, Apache Flink, and CrateDB (6a6912e)

docs/index.rst

proddata · 2023-03-22T16:37:07Z

docs/index.rst

+Overview
+========
+
+- This JDBC driver is needed in certain scenarios like the one outlined at


should we explain/give examples here?
I checked the Flink example repo, but it also is not clear there, why we should not use the PG JDBC driver

Yes, I've added the corresponding example with 6a6912e. But, true, I will be happy to add a more crystal-clear sentence if @matriv could provide a corresponding statement?

I don't know if we can do since we don't have that PR merged in the flink-jdbc-connector.
Maybe just open an issue to address this later once that PR is merged and the cratedb-flink-jobs example is in place.

Well, the example still works on the older versions of Apache Flink where the adjustments to the flink-jdbc-connector have not been needed, and points into the right direction why we need the "legacy" JDBC driver. This detail does not change, as we still need this driver in the future.

When apache/flink-connector-jdbc#29 is merged, we can add an additional reference here, right?

It also is not clear there, why we should not use the PG JDBC driver.
I will be happy to add a more crystal-clear sentence about the "why".

Coming from ¹², I think a good explanation could be:

When using the postgresql:// JDBC driver prefix, this will implicitly select the corresponding PostgreSQL dialect implementation for Apache Flink. However, this will employ a few behaviors that strictly expect a PostgreSQL server on the other end, so that some operations will croak on databases which only offer wire-compatibility with PostgreSQL.

Footnotes

https://github.com/crate/cratedb-flink-jobs/issues/3 ↩

https://pypi.org/project/sqlalchemy-postgresql-relaxed/ ↩

I've just added 159010b, in order to improve this detail further. What do you think about it?

Another example is the one from crate/crate#11608 (comment) but it may not be something to mention

I've added it to 1684ea5, thanks!

hlcianfagna · 2023-03-24T08:14:20Z

LICENSE

-
-   APPENDIX: How to apply the Apache License to your work.
-
-      To apply the Apache License to your work, attach the following


Can we remove this?

It got removed already?

hlcianfagna · 2023-03-24T08:15:39Z

docs/index.rst

+In turn, this will employ a few behaviours that strictly expect a PostgreSQL
+server on the other end, so that some operations will croak on databases
+offering wire-compatibility with PostgreSQL, but do not provide the
+corresponding features like the `hstore`_ or `jsonb`_ extensions.


"certain features" instead of "corresponding features" perhaps?

Addressed with 1684ea5, thanks!

hlcianfagna · 2023-03-24T08:17:29Z

docs/index.rst

+Overview
+========
+
+- This JDBC driver is needed in certain scenarios like the one outlined at


Another example is the one from crate/crate#11608 (comment) but it may not be something to mention

- Describe why it is still needed in a few cases. - Also add the recommendation to use the official driver in other cases.

In order to better educate users about "what's inside", this patch refactors a section of the documentation from the appendix to the front page.

matriv · 2023-03-24T15:31:09Z

docs/index.rst

+will implicitly select the corresponding dialect implementation for PostgreSQL.
+
+In turn, this will employ a few behaviours that strictly expect a PostgreSQL
+server on the other end, so that some operations will croak on databases


minor, I'd prefer: croak -> fail

Addressed with GH-369. Thanks!

matriv

Thank you @amotl !

amotl requested review from matriv and proddata March 22, 2023 16:33

amotl commented Mar 22, 2023

View reviewed changes

docs/index.rst Show resolved Hide resolved

proddata reviewed Mar 22, 2023

View reviewed changes

amotl requested review from proddata and hlcianfagna March 23, 2023 07:03

This was referenced Mar 23, 2023

Documentation: Improve/trim README crate/pgjdbc#43

Merged

Investigate differences between this driver and canonical PostgreSQL JDBC driver #353

Closed

hlcianfagna reviewed Mar 24, 2023

View reviewed changes

amotl force-pushed the amo/improve-docs branch from 159010b to 1684ea5 Compare March 24, 2023 11:05

amotl requested a review from hlcianfagna March 24, 2023 11:06

amotl mentioned this pull request Mar 24, 2023

Documentation: Add code snippet / synopsis to landing page #367

Merged

amotl enabled auto-merge (rebase) March 24, 2023 14:05

amotl added 6 commits March 24, 2023 15:53

Chore: Happy new year

1f9f681

Documentation: Clarify the purpose of this "legacy" driver

88ccce6

- Describe why it is still needed in a few cases. - Also add the recommendation to use the official driver in other cases.

Documentation: Make "implementation notes" more prominent

a648074

In order to better educate users about "what's inside", this patch refactors a section of the documentation from the appendix to the front page.

Documentation: Add example about Apache Kafka, Apache Flink, and CrateDB

beb5184

Documentation: Adjust link title to "basic connect" tutorial

bad3644

Documentation: Further clarify the scenario where this driver is needed

274bf0a

amotl force-pushed the amo/improve-docs branch from 1684ea5 to 274bf0a Compare March 24, 2023 14:53

hlcianfagna approved these changes Mar 24, 2023

View reviewed changes

amotl merged commit eda489f into master Mar 24, 2023

amotl deleted the amo/improve-docs branch March 24, 2023 14:56

matriv reviewed Mar 24, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve documentation to clarify purpose of the driver #365

Improve documentation to clarify purpose of the driver #365

amotl commented Mar 22, 2023 •

edited

Loading

proddata Mar 22, 2023

amotl Mar 22, 2023

matriv Mar 22, 2023

amotl Mar 22, 2023 •

edited

Loading

amotl Mar 23, 2023 •

edited

Loading

amotl Mar 23, 2023

hlcianfagna Mar 24, 2023

amotl Mar 24, 2023

hlcianfagna Mar 24, 2023

amotl Mar 24, 2023 •

edited

Loading

hlcianfagna Mar 24, 2023

amotl Mar 24, 2023

hlcianfagna Mar 24, 2023

matriv Mar 24, 2023

amotl Mar 24, 2023

matriv left a comment


		APPENDIX: How to apply the Apache License to your work.

		To apply the Apache License to your work, attach the following

Improve documentation to clarify purpose of the driver #365

Improve documentation to clarify purpose of the driver #365

Conversation

amotl commented Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amotl Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

amotl Mar 23, 2023 • edited Loading

Choose a reason for hiding this comment

Footnotes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amotl Mar 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matriv left a comment

Choose a reason for hiding this comment

amotl commented Mar 22, 2023 •

edited

Loading

amotl Mar 22, 2023 •

edited

Loading

amotl Mar 23, 2023 •

edited

Loading

amotl Mar 24, 2023 •

edited

Loading