-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Conversation
There was a problem hiding this 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.
@arikfr sure. Should it be two files or just one? how do I avoid duplicating logic? |
3054f52
to
7b4681c
Compare
@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. |
Hello! Yes, I will prepare everything this week. |
@spacentropy This might be useful: https://github.com/getredash/redash/wiki/Local-development-setup |
@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 :) |
7b4681c
to
a3308ad
Compare
Codecov ReportAttention: Patch coverage is
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
|
58ce196
to
9bdb86b
Compare
1085c72
to
5874cc3
Compare
b4091f1
to
7799f78
Compare
f46d05b
to
674ffa7
Compare
…d identity, add test
674ffa7
to
19d703b
Compare
@justinclift hi. I'm finally(lol) ready to merge this. |
What type of PR is this? (check all applicable)
Description
Hello. Here's the list of things that were improved: