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

document TableProperties fields with protocol links #519

Open
zachschuermann opened this issue Nov 21, 2024 · 0 comments
Open

document TableProperties fields with protocol links #519

zachschuermann opened this issue Nov 21, 2024 · 0 comments
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@zachschuermann
Copy link
Collaborator

should include more details and links to the protocol for each table property.

ref: #453 (comment)

@zachschuermann zachschuermann added the good first issue Good for newcomers label Nov 21, 2024
@zachschuermann zachschuermann added the documentation Improvements or additions to documentation label Nov 25, 2024
zachschuermann added a commit that referenced this issue Nov 25, 2024
## What changes are proposed in this pull request?
New `TableProperties` struct parses the `Metadata` action's
`configuration` into a strongly-typed configuration struct. Instead of
previously manually querying specific keys (e.g. checking ColumnMapping
key) we can leverage the configuration to appropriately handle DVs,
column mapping, etc. in a structured manner.

Pragmatically, the changes are:
1. New `TableProperties` struct along with new
`Snapshot::get_table_properties()` API
a. Note that every field is optional and additionally there is an
`unknown_properties` field which includes a hashmap of all fields that
did not parse to a known table property.
2. `table_properties` module including deserialization submodule to
implement functions to deserialize the fields of `TableProperties`

### Future work
- optimize/test/improve interval parsing:
#507
- introduce a facility to query protocol + table properties to unify
checks on e.g. column mapping mode/dv enablement/etc.
#508
- docs #519 
- #535 

## This PR affects the following public APIs
- New `TableProperties` and `Snapshot::get_table_properties` API

## How was this change tested?
new ut

Credit: @roeap for the original PR which this is based on #222
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant