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

feat: trigger table run & data preview directly from compiled web ui #77

Merged
merged 6 commits into from
Dec 22, 2024

Conversation

ashish10alex
Copy link
Owner

@ashish10alex ashish10alex commented Dec 21, 2024

The change is largely made to make the extension features more visible to a more casual user.

  • Able to run current file w/t dependencies & dependents
  • Able to preview results
  • test getDataformActionCmdFromActionList

Minor fixes for issues discovered while implementing this feature

  • If users active editor was compiled query web view and if they would have invoked a command to run current file previously it would not have been able to determine the active document
  • Avoid multiple invocations of api requests from web view if the user had switched active editor by implementing debounce

@ashish10alex ashish10alex self-assigned this Dec 21, 2024
@ashish10alex ashish10alex changed the title feat: trigger table run directly from compiled web ui feat: trigger table run & data preview directly from compiled web ui Dec 21, 2024
@HampB
Copy link
Collaborator

HampB commented Dec 21, 2024

Nice, addition @ashish10alex!
Should probably add an option to run with full-refresh aswell. Other than that it looks good to me.

off topic - Hopefully i'll get some more time over soon to help out more :-)

@ashish10alex
Copy link
Owner Author

thanks @HampB , thank you, added ability to full-refresh. Yh contribute when can, no pressure 🙂


I was curious if you had tried the external dependencies feature and if it works for you ?. It will show you the option to retrieve them when you click the dependencies and dependents ?

@HampB
Copy link
Collaborator

HampB commented Dec 21, 2024

The Dataplex linage? tbh, saw it first time now. But it looks interesting and seems to work when looking at tables with external dependencies, I will play around with it some.

Noticed that you cant collapse it once it's been opened, added a new bug issue for that.

@ashish10alex
Copy link
Owner Author

ashish10alex commented Dec 21, 2024

yup, it was a feature request request that randomly came about from within our team at work. It help to identify teams/people who might be using an output table that you have created. So incase you want to change the name of the table you have some estimate of impact and roughly know who to reach out to.

--

Makes me wonder should we consider renaming the parent component text to something else ? to make people aware that it shows external lineage too ?

@HampB
Copy link
Collaborator

HampB commented Dec 21, 2024

Yeah, i think it will be really useful in the daily.

Agree that the parent probably could get a better name, but I'm currently not sure what would be a good name. Maybe just simplify it to "Lineage" or "Data Lineage", but that's not optimal either.

In the future it would also be nice to merge the external deps from dataplex with the deps generated from dataform, to reduce the need of two lists. But we would then need some way to deal with users that does not have the dataplex api or permissions in a nice way.

@ashish10alex
Copy link
Owner Author

"Data Lineage" does not seem bad, makes the user want to click it. I have changed it to that. There are a few reasons why external dependents from the api might be kept separate.

  1. User requires specific permission as you already specified
  2. The api call is rather expensive ~800ms on my M2 machine. Also, since the operation has to happen after dry run it add substantial overhead if performed every time. Hence, I only invoke the api call when the user click the Dataplex section

@benjaminwestern
Copy link
Collaborator

This is a nice addition. Got a few things I would bolt on top of this but as it stands, lgtm!

@ashish10alex ashish10alex merged commit 187ef72 into main Dec 22, 2024
@ashish10alex ashish10alex deleted the trigger-run-webview branch January 11, 2025 21:25
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.

3 participants