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 Zoom views (DENG-6964) #1159

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Zoom views (DENG-6964) #1159

wants to merge 1 commit into from

Conversation

sean-rose
Copy link
Contributor

DENG-6964: Integrate Zoom into BigQuery/Looker using FiveTran (API)

After this is deployed I plan to manually add explores joining these views together in looker-spoke-default.

@sean-rose
Copy link
Contributor Author

The current generate-lookml CI error is likely because the views authorizations haven't been applied by data infra automation yet. I'll retry the CI tomorrow when the view authorizations will hopefully be in place.

glean_app: false
views:
meetings:
type: table_view
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to also generate table_explores for these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My plan is to have one main "meetings" explore joining several of these views together, and a "users" explore just on the users view. So while I could add a table_explore for the latter here, I figured since I'll be needing to do a looker-spoke-default PR for the other explore I'd just do both that way.

@sean-rose
Copy link
Contributor Author

The current generate-lookml CI error is likely because the views authorizations haven't been applied by data infra automation yet. I'll retry the CI tomorrow when the view authorizations will hopefully be in place.

The generate-lookml CI is still failing with the view authorizations in place. It appears what's happening is since these views select from a single table LookML generator is attempting to get information about that table to generate a datagroup for the Looker view, but it doesn't have permission to access the table.

@scholtzan do you have any advice on the best way to proceed here?

Two ideas that come to mind are:

  • Have LookML generator not try to generate datagroups for authorized views. However, detecting authorized views would require changing the dryrun logic both here and in the cloud function to return tables' labels.
  • Have LookML generator ignore permission errors.

@scholtzan
Copy link
Contributor

I thought we had some logic in place already to ignore permission errors...

@scholtzan
Copy link
Contributor

It should hit this:

if e.error == Errors.PERMISSION_DENIED and e.use_cloud_function:

@sean-rose
Copy link
Contributor Author

It should hit this:

if e.error == Errors.PERMISSION_DENIED and e.use_cloud_function:

Maybe the issue is that the original DryRunError is getting caught and a ValueError is getting raised with the original exception as the cause:

except DryRunError as e:
raise ValueError(
f"Unable to find {source_table_id} referenced in {full_table_id}"
) from e

@scholtzan
Copy link
Contributor

This should fix it: #1161

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

Successfully merging this pull request may close these issues.

2 participants