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

Add athena ref page #5961

Merged
merged 34 commits into from
Aug 29, 2024
Merged

Add athena ref page #5961

merged 34 commits into from
Aug 29, 2024

Conversation

matthewshaver
Copy link
Contributor

What are you changing in this pull request and why?

Adding the Amazon Athena /ref page
Additionally organized the adapter ref page sidebar in alphabetical order

Checklist

Adding or removing pages (delete if not applicable):

  • Add/remove page in website/sidebars.js
  • Provide a unique filename for new pages
  • Add an entry for deleted pages in website/vercel.json
  • Run link testing locally with npm run build to update the links that point to deleted pages

@matthewshaver matthewshaver requested review from dataders and a team as code owners August 22, 2024 15:57
Copy link

vercel bot commented Aug 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs-getdbt-com ✅ Ready (Inspect) Visit Preview Aug 29, 2024 8:57pm

@github-actions github-actions bot added content Improvements or additions to content Docs team Authored by the Docs team @dbt Labs size: large This change will more than a week to address and might require more than one person and removed Docs team Authored by the Docs team @dbt Labs labels Aug 22, 2024
@github-actions github-actions bot added the Docs team Authored by the Docs team @dbt Labs label Aug 22, 2024
@matthewshaver matthewshaver changed the title Addint athena ref page Add athena ref page Aug 22, 2024
- `data_cell_filters` management can't be automated outside dbt because the filter can't be attached to the table which doesn't exist. Once you `enable` this config, dbt will set all filters and their permissions during every dbt run. Such approach keeps the actual state of row level security configuration actual after every dbt run and apply changes if they occur: drop, create, update filters and their permissions.
- Any tags listed in `lf_inherited_tags` should be strictly inherited from the database level and never overridden at the table and column level
- Currently `dbt-athena` does not differentiate between an inherited tag association and an override of same it made previously
> - For example, If an inherited tag is overridden by an `lf_tags_config` value in one DBT run, and that override is removed prior to a subsequent run, the prior override will linger and no longer be encoded anywhere (in e.g. Terraform where the inherited value is configured nor in the DBT project where the override previously existed but now is gone)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
> - For example, If an inherited tag is overridden by an `lf_tags_config` value in one DBT run, and that override is removed prior to a subsequent run, the prior override will linger and no longer be encoded anywhere (in e.g. Terraform where the inherited value is configured nor in the DBT project where the override previously existed but now is gone)
> - For example, If an inherited tag is overridden by an `lf_tags_config` value in one dbt run, and that override is removed prior to a subsequent run, the prior override will linger and no longer be encoded anywhere (in e.g. Terraform where the inherited value is configured nor in the dbt project where the override previously existed but now is gone)

The materialization also supports invalidating hard deletes. Check
the [docs](https://docs.getdbt.com/docs/build/snapshots#hard-deletes-opt-in) to understand usage.

### Working example
Copy link
Collaborator

@amychen1776 amychen1776 Aug 22, 2024

Choose a reason for hiding this comment

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

Please delete the working example. I don't think we need it

Copy link
Contributor

@nghi-ly nghi-ly left a comment

Choose a reason for hiding this comment

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

excited to see this new adapter page! here's the first chunk of my review (the Models section and all of its subsections). gonna tackle the Snapshots section next.


| Parameter | Default | Description |
|-----------|---------|-------------|
| `external_location` | None | The full S3 path to where the table will be saved. Only works with incremental models. Doesn't work with Hive table with `ha` set to `true`. |
Copy link
Contributor

@nghi-ly nghi-ly Aug 22, 2024

Choose a reason for hiding this comment

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

  • sugg: rewording first sentence so it's active voice (vs passive)
  • sugg: is the indefinite article missing in the 3rd sentence?

website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
'disabled' as status
```

By default, the materialization keeps the last 4 table versions,but you can change it by setting `versions_to_keep`.
Copy link
Contributor

Choose a reason for hiding this comment

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

sugg moving this sentence to above the code block. feels like this could get lost in the scanning


### Update glue data catalog

Persist resource descriptions as column and relation comments to the glue data catalog, and meta as [glue table properties](https://docs.aws.amazon.com/glue/latest/dg/tables-described.html#table-properties) and [column parameters](https://docs.aws.amazon.com/glue/latest/webapi/API_Column.html). By default, documentation persistence is disabled, but it can be enabled for specific resources or groups of resources as needed.
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm having a hard time reading and grokking the first sentence. sugg rewording to improve clarity

sugg updating second sentence to be in active voice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm struggling here too. @amychen1776 Any ideas on how to clean this thought up?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe something like this:

You can persist your column and model level descriptions to the Glue Data Catalog as glue table properties and column parameters. To enable this, you will need to set the configuration to true as show below.

website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
Copy link
Contributor

@nghi-ly nghi-ly left a comment

Choose a reason for hiding this comment

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

here's my review of the H2 Snapshots section and all of its subsections. gonna tackle the next H2.

Copy link
Contributor

@nghi-ly nghi-ly left a comment

Choose a reason for hiding this comment

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

last chunk of my review!

website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
website/docs/reference/resource-configs/athena-configs.md Outdated Show resolved Hide resolved
matthewshaver and others added 2 commits August 29, 2024 13:46
Copy link
Collaborator

@amychen1776 amychen1776 left a comment

Choose a reason for hiding this comment

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

:shipit:

@matthewshaver matthewshaver enabled auto-merge (squash) August 29, 2024 20:54
@matthewshaver matthewshaver merged commit 54610a8 into current Aug 29, 2024
10 checks passed
@matthewshaver matthewshaver deleted the athena-reference branch August 29, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Improvements or additions to content Docs team Authored by the Docs team @dbt Labs size: large This change will more than a week to address and might require more than one person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants