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

Data catalog Tag module #2060

Merged
merged 7 commits into from
Feb 13, 2024
Merged

Data catalog Tag module #2060

merged 7 commits into from
Feb 13, 2024

Conversation

lcaggio
Copy link
Collaborator

@lcaggio lcaggio commented Feb 8, 2024

Data catalog Tag module. This is a simple module. The main reason for creating a module is the Tag factory.


Checklist

I applicable, I acknowledge that I have:

  • Read the contributing guide
  • Ran terraform fmt on all modified files
  • Regenerated the relevant README.md files using tools/tfdoc.py
  • Made sure all relevant tests pass

@lcaggio lcaggio marked this pull request as ready for review February 8, 2024 19:05
@lcaggio lcaggio requested review from juliocc and ludoo February 12, 2024 21:45
@thinhha
Copy link
Contributor

thinhha commented Feb 13, 2024

This looks great! Thanks Lorenzo.

@juliocc juliocc merged commit aa9b147 into master Feb 13, 2024
13 checks passed
@juliocc juliocc deleted the lcaggio/datcat-tag branch February 13, 2024 16:24
Copy link
Collaborator

@wiktorn wiktorn left a comment

Choose a reason for hiding this comment

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

I wonder about actual usage. @lcaggio based on your experience - is it multiple templates applied to single parent, or rather one template per parent?

Also, can template location can be different than tag location?

I wonder about remodeling this structure a bit, to reduce boilerplate code, like for example if I have multiple column on one parent to tag


resource "google_data_catalog_tag" "engine" {
for_each = local.factory_tag_template
parent = "projects/${each.value.project_id}/locations/${each.value.project_id}/entryGroups/@bigquery/entries/${trim(base64encode(each.value.parent), "=")}"
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
parent = "projects/${each.value.project_id}/locations/${each.value.project_id}/entryGroups/@bigquery/entries/${trim(base64encode(each.value.parent), "=")}"
parent = "projects/${each.value.project_id}/locations/${each.value.location}/entryGroups/@bigquery/entries/${trim(base64encode(each.value.parent), "=")}"

project_id = "project-data-product"
parent = "projects/project-data-product/datasets/landing/tables/countries"
column = "country"
location = "europe-west-1"
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
location = "europe-west-1"
location = "europe-west1"

?

@lcaggio
Copy link
Collaborator Author

lcaggio commented Feb 13, 2024

Thanks @wiktorn. From my experience, multiple Tag can be assigned to a resource (e.g. multiple tag on a dataset, less common for columns).

I think location can be different, but I am going to use this module in 2 customers really soon and I will check the structure of the module with reality of customer needs. I will update the module accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants