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

Update Azure Data Explorer query runner to latest version, add managed identity #5495

Closed
wants to merge 5 commits into from

Conversation

spacentropy
Copy link
Contributor

@spacentropy spacentropy commented May 10, 2021

What type of PR is this? (check all applicable)

  • Refactor

Description

Hello. Here's the list of things that were improved:

  • upgraded Azure Data Explorer(Kusto) library to the latest version
  • added Materialized Views to schema viewer
  • added Managed Identities support for secrets-less auth
  • added query user annotation in request description
  • added data_scanned
  • added simple test to get_schema

requirements_all_ds.txt Outdated Show resolved Hide resolved
Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! How about we split the query runner into two: one for MSI and one for without? This way it will be clear which properties are required for each variant.

@spacentropy
Copy link
Contributor Author

@arikfr sure. Should it be two files or just one? how do I avoid duplicating logic?

@spacentropy spacentropy force-pushed the upd-azure-kusto-2-0 branch from 3054f52 to 7b4681c Compare April 11, 2023 06:47
@guidopetri
Copy link
Contributor

@spacentropy , thanks for the PR! We've updated a lot of things now that we're Community-driven so - if you're still interested in getting this merged - would you mind rebasing off master to re-run the CI?

We're trying to clean up our PR todo list, so if you're not interested, that's fine - we'll close the PR in about a week if we don't hear back. If you're interested in reopening the PR afterwards, we would also very much welcome that.

@spacentropy
Copy link
Contributor Author

Hello! Yes, I will prepare everything this week.

@justinclift
Copy link
Member

@guidopetri
Copy link
Contributor

@spacentropy no worries, take your time. We just wanted to make sure this wasn't an abandoned PR - if it takes a month for you to get around to it it's also fine :)

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Attention: Patch coverage is 11.76471% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 63.76%. Comparing base (af0773c) to head (674ffa7).
Report is 42 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5495      +/-   ##
==========================================
- Coverage   63.82%   63.76%   -0.06%     
==========================================
  Files         161      161              
  Lines       13060    13085      +25     
  Branches     1803     1810       +7     
==========================================
+ Hits         8335     8344       +9     
- Misses       4425     4437      +12     
- Partials      300      304       +4     
Files Coverage Δ
redash/query_runner/azure_kusto.py 35.13% <11.76%> (-3.11%) ⬇️

@spacentropy spacentropy force-pushed the upd-azure-kusto-2-0 branch 2 times, most recently from 58ce196 to 9bdb86b Compare December 6, 2023 11:52
@spacentropy spacentropy force-pushed the upd-azure-kusto-2-0 branch 2 times, most recently from 1085c72 to 5874cc3 Compare January 11, 2024 21:55
@spacentropy spacentropy force-pushed the upd-azure-kusto-2-0 branch from b4091f1 to 7799f78 Compare April 19, 2024 09:32
@spacentropy spacentropy force-pushed the upd-azure-kusto-2-0 branch from f46d05b to 674ffa7 Compare May 8, 2024 10:49
@spacentropy spacentropy marked this pull request as ready for review October 15, 2024 06:48
@spacentropy spacentropy marked this pull request as draft October 15, 2024 06:49
@spacentropy spacentropy marked this pull request as ready for review October 15, 2024 10:26
@spacentropy
Copy link
Contributor Author

@justinclift hi. I'm finally(lol) ready to merge this.

@spacentropy
Copy link
Contributor Author

@eradman @arikfr Hello. I would like to merge this :-) We've been running this branch for a while.

@spacentropy spacentropy requested a review from arikfr October 18, 2024 08:06
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.

5 participants