Docs: Location Provider Documentation by smaheshwar-pltr · Pull Request #1537 · apache/iceberg-python

smaheshwar-pltr · 2025-01-18T13:57:44Z

(See below for screenshots)

Closes #1510. This is my first time writing docs here! Happy to receive style feedback - I already suspect I've written too much.

cc @kevinjqliu @Fokko

smaheshwar-pltr · 2025-01-18T16:17:11Z

mkdocs/docs/configuration.md

+| `write.metadata.previous-versions-max`   | Integer                           | 100     | The max number of previous version metadata files to keep before deleting after commit.                                             |
+| `write.object-storage.enabled`           | Boolean                           | True    | Enables the [ObjectStoreLocationProvider](configuration.md#objectstorelocationprovider) that adds a hash component to file paths    |
+| `write.object-storage.partitioned-paths` | Boolean                           | True    | Controls whether [partition values are included in file paths](configuration.md#partition-exclusion) when object storage is enabled |
+| `write.py-location-provider.impl`        | String of form `module.ClassName` | null    | Optional, [custom LocationProvider](configuration.md#loading-a-custom-locationprovider) implementation                              |


Unsure about what Options to put here. Happy to take suggestions.

maybe similar to the custom catalog
https://py.iceberg.apache.org/configuration/#custom-catalog-implementations

Don't love how this looks. I prefer what it is now:

I've changed the section linked above to have mymodule.MyLocationProvider in custom Catalog style. WDYT?

(The above screenshot also shows how code/backticks hyperlinks look, I think they're fine. This is now relevant because of #1537 (comment).

mkdocs/docs/configuration.md

smaheshwar-pltr · 2025-01-18T16:21:45Z

mkdocs/docs/configuration.md

+
+### SimpleLocationProvider
+
+The SimpleLocationProvider places file names underneath a `data` directory in the table's storage location. For example,


I realised I was wrong in #1510 (comment) about docs not needing to change when write.data.path is supported. I think we do want to say that data files are under a data directory, here and below.

But I think this is fine because the change will be small - it'd just be "write.data.path" instead of "data directory under the table's location" throughout. write.data.path defaults to this.

smaheshwar-pltr · 2025-01-18T16:24:51Z

mkdocs/docs/configuration.md

+
+The SimpleLocationProvider is enabled for a table by explicitly setting its `write.object-storage.enabled` table property to `false`.
+
+### ObjectStoreLocationProvider


There's a lot of natural duplication between this section and https://iceberg.apache.org/docs/latest/aws/#object-store-file-layout. I've gone less in-depth here though.

I was unsure whether to link to this webpage (and if so, how to word it) because there's a lot that's not relevant to us, e.g.

Note, the path resolution logic for ObjectStoreLocationProvider is write.data.path then /data. However, for the older versions up to 0.12.0, the logic is as follows: - before 0.12.0, write.object-storage.path must be set. - at 0.12.0, write.object-storage.path then write.folder-storage.path then /data.

and

Previously provided base64 hash was updated to base2 in order to provide an improved auto-scaling behavior on S3 General Purpose Buckets.

i think we should link to that for additional context

smaheshwar-pltr · 2025-01-18T16:25:18Z

mkdocs/docs/configuration.md

+s3://bucket/ns/table/data/1101/0100/1011/00111010-00000-0-5affc076-96a4-48f2-9cd2-d5efbc9f0c94-00001.parquet
+```
+
+### Loading a Custom LocationProvider


Similar to https://iceberg.apache.org/docs/latest/custom-catalog/#custom-location-provider-implementation

smaheshwar-pltr · 2025-01-18T16:27:08Z

pyiceberg/table/locations.py


 class LocationProvider(ABC):
-    """A base class for location providers, that provide data file locations for write tasks."""
+    """A base class for location providers, that provide data file locations for a table's write tasks.


Added these docs so folks looking to write a custom LocationProvider see

smaheshwar-pltr · 2025-01-18T16:31:31Z

mkdocs/docs/configuration.md

+### Loading a Custom LocationProvider
+
+Similar to FileIO, a custom LocationProvider may be provided for a table by concretely subclassing the abstract base
+class [LocationProvider](../reference/pyiceberg/table/locations/#pyiceberg.table.locations.LocationProvider). The


I wanted to link to this, and this works for me locally, but I get the following warning when serving docs locally:

INFO - Doc file 'configuration md' contains an unrecognized relative link '.. / reference/pyiceberg/table/locations/#pyiceberg.table.locations.LocationProvider', it was left as is. Did you mean ' reference/pyiceberg/table/locations.md#pyiceberg.table.locations.LocationProvider'?

But I get a similar warning elsewhere: Doc file 'SUMMARY.md' contains an unrecognized relative link 'reference/', it was left as is.. So I think this is fine?

yea i think its fine, as long as the hyperlink works when you run it locally

smaheshwar-pltr · 2025-01-18T16:34:44Z

Here are screenshots of (current) changes. I've verified that all the new links work.

(Edit: I've since made minor spelling changes. After this PR is reviewed, I'll post the up-to-date screenshots, so folks can see what'll be merged, instead of posting on every change)

smaheshwar-pltr · 2025-01-18T21:30:05Z

mkdocs/docs/configuration.md

+An example, custom `LocationProvider` implementation is shown below.
+
+```py
+import uuid


I've only shown this import for conciseness.

smaheshwar-pltr · 2025-01-19T10:59:27Z

mkdocs/docs/configuration.md

+| `write.parquet.page-row-limit`           | Number of rows                    | 20000   | Set a target threshold for the maximum number of rows within a column chunk                                                         |
+| `write.parquet.dict-size-bytes`          | Size in bytes                     | 2MB     | Set the dictionary page size limit per row group                                                                                    |
+| `write.metadata.previous-versions-max`   | Integer                           | 100     | The max number of previous version metadata files to keep before deleting after commit.                                             |
+| `write.object-storage.enabled`           | Boolean                           | True    | Enables the [ObjectStoreLocationProvider](configuration.md#objectstorelocationprovider) that adds a hash component to file paths    |


(Same wording, more or less, as https://iceberg.apache.org/docs/latest/configuration/#write-properties)

maybe we should add a warning or something about how this default differs from the java implementation

smaheshwar-pltr · 2025-01-19T11:14:54Z

mkdocs/docs/configuration.md

+The ObjectStoreLocationProvider counteracts this by injecting deterministic hashes, in the form of binary directories,
+into file paths, to distribute files across a larger number of object store prefixes.
+
+Paths contain partitions just before the file name and a `data` directory beneath the table's location, in a similar


See #1537 (comment) re data here.

kevinjqliu

Thanks for the PR! These docs are great. I've added a few nit comments

kevinjqliu · 2025-01-19T19:34:45Z

mkdocs/docs/configuration.md

+| `write.metadata.previous-versions-max`   | Integer                           | 100     | The max number of previous version metadata files to keep before deleting after commit.                                             |
+| `write.object-storage.enabled`           | Boolean                           | True    | Enables the [ObjectStoreLocationProvider](configuration.md#objectstorelocationprovider) that adds a hash component to file paths    |
+| `write.object-storage.partitioned-paths` | Boolean                           | True    | Controls whether [partition values are included in file paths](configuration.md#partition-exclusion) when object storage is enabled |
+| `write.py-location-provider.impl`        | String of form `module.ClassName` | null    | Optional, [custom LocationProvider](configuration.md#loading-a-custom-locationprovider) implementation                              |


maybe similar to the custom catalog
https://py.iceberg.apache.org/configuration/#custom-catalog-implementations

kevinjqliu · 2025-01-19T19:38:54Z

mkdocs/docs/configuration.md

+| `write.parquet.page-row-limit`           | Number of rows                    | 20000   | Set a target threshold for the maximum number of rows within a column chunk                                                         |
+| `write.parquet.dict-size-bytes`          | Size in bytes                     | 2MB     | Set the dictionary page size limit per row group                                                                                    |
+| `write.metadata.previous-versions-max`   | Integer                           | 100     | The max number of previous version metadata files to keep before deleting after commit.                                             |
+| `write.object-storage.enabled`           | Boolean                           | True    | Enables the [ObjectStoreLocationProvider](configuration.md#objectstorelocationprovider) that adds a hash component to file paths    |


maybe we should add a warning or something about how this default differs from the java implementation

kevinjqliu · 2025-01-19T19:39:19Z

mkdocs/docs/configuration.md

+| `write.parquet.dict-size-bytes`          | Size in bytes                     | 2MB     | Set the dictionary page size limit per row group                                                                                    |
+| `write.metadata.previous-versions-max`   | Integer                           | 100     | The max number of previous version metadata files to keep before deleting after commit.                                             |
+| `write.object-storage.enabled`           | Boolean                           | True    | Enables the [ObjectStoreLocationProvider](configuration.md#objectstorelocationprovider) that adds a hash component to file paths    |
+| `write.object-storage.partitioned-paths` | Boolean                           | True    | Controls whether [partition values are included in file paths](configuration.md#partition-exclusion) when object storage is enabled |


hyperlinks are weird sometimes, can you make sure that these work as intended

I've checked all hyperlinks on the current version and they work as intended

mkdocs/docs/configuration.md

kevinjqliu · 2025-01-19T19:47:35Z

mkdocs/docs/configuration.md

+
+### SimpleLocationProvider
+
+The SimpleLocationProvider places file names underneath a `data` directory in the table's storage location. For example,


Suggested change

The SimpleLocationProvider places file names underneath a `data` directory in the table's storage location. For example,

The `SimpleLocationProvider` places a table's data file names underneath a `data` directory in the table's storage location. For example,

I think we should also call out that the "base location" is the table.metadata.location

From the spec, https://iceberg.apache.org/spec/#table-metadata-fields

The table's base location. This is used by writers to determine where to store data files, manifest files, and table metadata files.

kevinjqliu · 2025-01-19T19:49:45Z

mkdocs/docs/configuration.md

+s3://bucket/ns/table/data/0000-0-5affc076-96a4-48f2-9cd2-d5efbc9f0c94-00001.parquet
+```
+
+When data is partitioned, files under a given partition are grouped into a subdirectory, with that partition key


Suggested change

When data is partitioned, files under a given partition are grouped into a subdirectory, with that partition key

When the table is partitioned, files under a given partition are grouped into a subdirectory, with that partition key

nit: maybe also when the hive-style partition path

Hopefully this and this is wha you meant

kevinjqliu · 2025-01-19T19:52:55Z

mkdocs/docs/configuration.md

+```txt
+s3://bucket/ns/table/data/1101/0100/1011/00111010-00000-0-5affc076-96a4-48f2-9cd2-d5efbc9f0c94-00001.parquet
+```


nit: what about giving an example of this set to True and another one set to False

I have the False case just above ("the same data file above" here) - or do you mean making that more explicit?

i think given the new wording above, its clear now :) thanks!

kevinjqliu · 2025-01-19T19:53:23Z

mkdocs/docs/configuration.md

+
+The SimpleLocationProvider is enabled for a table by explicitly setting its `write.object-storage.enabled` table property to `false`.
+
+### ObjectStoreLocationProvider


i think we should link to that for additional context

kevinjqliu · 2025-01-19T19:54:39Z

mkdocs/docs/configuration.md

+### Loading a Custom LocationProvider
+
+Similar to FileIO, a custom LocationProvider may be provided for a table by concretely subclassing the abstract base
+class [LocationProvider](../reference/pyiceberg/table/locations/#pyiceberg.table.locations.LocationProvider). The


yea i think its fine, as long as the hyperlink works when you run it locally

kevinjqliu · 2025-01-19T19:55:16Z

mkdocs/docs/configuration.md

+class [LocationProvider](../reference/pyiceberg/table/locations/#pyiceberg.table.locations.LocationProvider). The
+table property `write.py-location-provider.impl` should be set to the fully-qualified name of the custom
+LocationProvider (i.e. `module.CustomLocationProvider`). Recall that a LocationProvider is configured per-table,
+permitting different location provision for different tables.


also mention that java uses write.location-provider.impl

kevinjqliu

LGTM! Thanks for the great docs

kevinjqliu · 2025-01-20T18:31:44Z

mkdocs/docs/configuration.md

+```txt
+s3://bucket/ns/table/data/1101/0100/1011/00111010-00000-0-5affc076-96a4-48f2-9cd2-d5efbc9f0c94-00001.parquet
+```


i think given the new wording above, its clear now :) thanks!

Fokko

This looks great @smaheshwar-pltr thanks for the comprehensive documentation.

smaheshwar-pltr · 2025-01-24T13:44:41Z

This PR concludes initial support for location providers in PyIceberg 🎉

Huge thanks to Kevin and Fokko for their excellent and patient reviews throughout these PRs - I really appreciate your responsiveness and focus on quality that made this contribution enjoyable. Thank you for that and for all your work with PyIceberg!

Sreesh Maheshwar added 2 commits January 18, 2025 13:57

Documentation for Location Providers

ef82985

Finish docs

3b94570

smaheshwar-pltr marked this pull request as ready for review January 18, 2025 16:16

smaheshwar-pltr commented Jan 18, 2025

View reviewed changes

mkdocs/docs/configuration.md Outdated Show resolved Hide resolved

smaheshwar-pltr commented Jan 18, 2025

View reviewed changes

smaheshwar-pltr mentioned this pull request Jan 18, 2025

Documentation for Location Providers #1510

Closed

smaheshwar-pltr changed the title ~~Documentation for Location Providers~~ Docs: Location Provider Documentation Jan 18, 2025

smaheshwar-pltr commented Jan 18, 2025

View reviewed changes

smaheshwar-pltr commented Jan 19, 2025

View reviewed changes

Minor spelling fixes

3ee2695

smaheshwar-pltr commented Jan 19, 2025

View reviewed changes

kevinjqliu reviewed Jan 19, 2025

View reviewed changes

Sreesh Maheshwar added 3 commits January 19, 2025 22:09

Address some comments

6be752a

Address all comments

76f397b

Fix all hyperlinks

5ee6dec

smaheshwar-pltr requested a review from kevinjqliu January 19, 2025 23:02

kevinjqliu approved these changes Jan 20, 2025

View reviewed changes

kevinjqliu requested a review from Fokko January 20, 2025 18:32

Fokko approved these changes Jan 24, 2025

View reviewed changes

Fokko merged commit a93e300 into apache:main Jan 24, 2025
8 checks passed

smaheshwar-pltr deleted the location-providers-docs branch January 24, 2025 13:38

smaheshwar-pltr mentioned this pull request Feb 5, 2025

Add support for write.data.path #1611

Merged


		### SimpleLocationProvider

		The SimpleLocationProvider places file names underneath a `data` directory in the table's storage location. For example,


		The SimpleLocationProvider is enabled for a table by explicitly setting its `write.object-storage.enabled` table property to `false`.

		### ObjectStoreLocationProvider

	The SimpleLocationProvider places file names underneath a `data` directory in the table's storage location. For example,
	The `SimpleLocationProvider` places a table's data file names underneath a `data` directory in the table's storage location. For example,

	When data is partitioned, files under a given partition are grouped into a subdirectory, with that partition key
	When the table is partitioned, files under a given partition are grouped into a subdirectory, with that partition key

Conversation

smaheshwar-pltr commented Jan 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smaheshwar-pltr Jan 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smaheshwar-pltr commented Jan 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinjqliu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinjqliu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fokko left a comment

smaheshwar-pltr commented Jan 18, 2025 •

edited

Loading

smaheshwar-pltr Jan 18, 2025 •

edited

Loading

smaheshwar-pltr commented Jan 18, 2025 •

edited

Loading