Document the base arrow module#23212
Document the base arrow module#23212SthuthiGhosh9400 wants to merge 20 commits intoprestodb:masterfrom
Conversation
steveburnett
left a comment
steveburnett
left a comment
There was a problem hiding this comment.
Thanks for the doc! Some suggestions of phrasing and formatting to consider.
steveburnett
left a comment
steveburnett
left a comment
There was a problem hiding this comment.
Transferred yesterday's unaddressed comments to today's, as some of yesterday's were hidden behind a fold. Most of these are formatting suggestions.
Also, please add
connector/base-arrow-flight to
https://github.com/prestodb/presto/blob/master/presto-docs/src/main/sphinx/connector.rst
so the new page is present on the Connector page in the Presto documentation.
Add the new page in alphabetic order for the new page title Arrow-flight Connector.
|
|
||
| Note : You can add other properties that are required for your Flight server to connect. | ||
|
|
||
| ========================================== ============================================================== |
There was a problem hiding this comment.
Suggest adding a third column to this table for Default values.
For one example, should arrow-flight.server.port have a default value of 32010?
For items that don't have a default, such as data-source.host, leave the column blank for that row.
|
steveburnett
left a comment
steveburnett
left a comment
There was a problem hiding this comment.
-
Local doc build returns the following warning:
/Users/steveburnett/Documents/GitHub/presto/presto-docs/src/main/sphinx/connector/base-arrow-flight.rst: WARNING: document isn't included in any toctreeTo address this warning, add
connector/base-arrow-flightto
https://github.com/prestodb/presto/blob/master/presto-docs/src/main/sphinx/connector.rst
so the new page is present on the Connector page in the Presto documentation.Add the new page in alphabetic order for the new page title Arrow-flight Connector.
-
Please sign the CLA as mentioned in this comment.
-
Suggest release note entry following the Release Notes Guidelines:
== RELEASE NOTES == General Changes * Add :doc:`../connector/base-arrow-flight` documentation :pr:`23212` -
When the comments have been addressed, please squash commits as mentioned in the Pull Request Guidelines.
| Apache Arrow enhances performance and efficiency in data-intensive applications through its columnar memory layout, zero-copy reads, vectorized execution, cross-language interoperability, rich data type support, and optimization for modern hardware. These features collectively reduce overhead, improve data processing speeds, and facilitate seamless data exchange between different systems and languages. | ||
|
|
||
| Getting Started with base-arrow-module: Essential Abstract Methods for Developers | ||
| -------------- |
There was a problem hiding this comment.
| -------------- | |
| --------------------------------------------------------------------------------- | |
The formatting -------- must be as long as the text in the preceding line. Currently, a warning is generated in the local doc build as follows:
/Users/steveburnett/Documents/GitHub/presto/presto-docs/src/main/sphinx/connector/base-arrow-flight.rst:9: WARNING: Title underline too short.
|
|
||
|
|
||
| Arrow-Flight Connector Limitations | ||
| --------------------------------- |
There was a problem hiding this comment.
| --------------------------------- | |
| ---------------------------------- |
Warning in local doc build:
/Users/steveburnett/Documents/GitHub/presto/presto-docs/src/main/sphinx/connector/base-arrow-flight.rst:100: WARNING: Title underline too short.
| data-source.port=<PORT> | ||
| data-source.ssl=<TRUE/FALSE> | ||
|
|
||
| Add other properties that are required for your Flight server to connect. |
There was a problem hiding this comment.
| Add other properties that are required for your Flight server to connect. | |
| Add other properties that are required for your Flight server to connect. |
Formatting suggestion, because this sentence does not appear to be part of the code block.
| Querying Arrow-Flight | ||
| --------------------- | ||
|
|
||
| The Arrow-Flight connector provides schema for each supported *databases*. |
There was a problem hiding this comment.
| The Arrow-Flight connector provides schema for each supported *databases*. | |
| The Arrow-Flight connector provides schema for each supported *database*. |
| Arrow-Flight Connector Limitations | ||
| --------------------------------- | ||
|
|
||
| * SELECT and DESCRIBE queries are supported by this connector template. Implementing modules can add support for additional features. |
There was a problem hiding this comment.
| * SELECT and DESCRIBE queries are supported by this connector template. Implementing modules can add support for additional features. | |
| * :doc:`/sql/select` and :doc:`/sql/describe` queries are supported by this connector template. Implementing modules can add support for additional features. |
|
If the documentation for #23032 is being added in this PR, suggest a release note entry for this PR as follows: and removing from the release note entry of #23032. |
|
Because the documentation for the Arrow connector appears to be being added in #23032 with the connector code, unless I'm overlooking something here, I suggest closing this PR. |
|
Closing this as the doc for Arrow was merged in #24427 |
2 participants
Description
Motivation and Context
Impact
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.