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

Modernize henry - fixes all current bugs and switches to non-deprecated system__activity #85

Merged
merged 4 commits into from
Apr 20, 2024

Conversation

rbob86
Copy link
Collaborator

@rbob86 rbob86 commented Apr 3, 2024

Fixes crash due to outdated object reference
#82

Fixes crash due to invalid field reference in call to run_inline_query in get_used_models()
#86

Fixes invalid filter error due to i__looker losing workspace_id filter
#83

Adds looker__internal_analytics__replica to reserved_names in check_db_connection

Incorporates changes from #57 to use system__activity instead of i__looker, which is deprecated.

@rbob86 rbob86 requested review from josephaxisa and a team as code owners April 3, 2024 18:38
@jeremytchang
Copy link
Contributor

Can you confirm that you've verified this fixes the issue? Also can you link the issue in the PR description?

@rbob86
Copy link
Collaborator Author

rbob86 commented Apr 3, 2024

Can you confirm that you've verified this fixes the issue? Also can you link the issue in the PR description?

This resolves the issue (see first output of python -m henry.cli --help without change, second with the change:

image

@ice-ctrl
Copy link

ice-ctrl commented Apr 5, 2024

Please merge this update. It resolves one of the issues at least.

@rbob86 rbob86 changed the title Updated invalid looker_sdk models object reference in pulse.py Modernize henry - fixes all current bugs Apr 12, 2024
@rbob86 rbob86 changed the title Modernize henry - fixes all current bugs Modernize henry - fixes all current bugs and switches to non-deprecated system__activity Apr 12, 2024
Copy link
Collaborator

@josephaxisa josephaxisa left a comment

Choose a reason for hiding this comment

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

all changes look reasonable, thanks for the PR @rbob86

@josephaxisa josephaxisa merged commit edef64d into looker-open-source:master Apr 20, 2024
1 check passed
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.

4 participants