Metrica: retry query if quota exceeded

[denisov-vlad]

What type of PR is this?

• Refactor
• Feature
• Bug Fix
• New Query Runner (Data Source)
• New Alert Destination
• Other

Description

Metrica has a limit for simultaneous queries. If you have dashboard with 5+ different queries there is always errors. I use backoff library to retry queries if it fails with 429 (quota exceeded) error.

How is this tested?

• Unit tests (pytest, jest)
• E2E Tests (Cypress)
• Manually
• N/A

Ran queries with modified source both within query and dashboard.

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

[codecov]

Codecov Report

Merging #6459 (d80609b) into master (d4ade51) will increase coverage by 0.27%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #6459      +/-   ##
==========================================
+ Coverage   61.26%   61.53%   +0.27%     
==========================================
  Files         158      158              
  Lines       12889    12898       +9     
  Branches     1755     1756       +1     
==========================================
+ Hits         7896     7937      +41     
+ Misses       4743     4704      -39     
- Partials      250      257       +7     

Files	Coverage Δ
redash/query_runner/yandex_metrica.py	70.43% <66.66%> (+32.69%)	⬆️

[guidopetri]

This makes sense to me (although ideally we'd just submit the queries to Metrica in sequence and not overload it). Can we add a test of some kind to verify the quota exceeded error gets raised, and that the query gets retried after a while?

[denisov-vlad]

@guidopetri

although ideally we'd just submit the queries to Metrica in sequence and not overload it

Yes, but in this case we need to change the logic for the scheduler/worker.

Can we add a test of some kind to verify the quota exceeded error gets raised, and that the query gets retried after a while?

That's not possible without access token.

[guidopetri]

Hmm, is there no way to mock the calls and have a mocked object keep track of how many active queries there are?

[denisov-vlad]

Honestly, I'm not sure that I can create unit test with parallel execution, because I have small experience with pytest. If you have suggestions how to do it, please share with me, and I'll try to research.

[guidopetri]

I don't think we'd need parallel execution. Maybe we can set it up as a fixture that looks something like this:

def keep_only_5_items(item):
    if len(active_items) >= 5:
        print('Only 5 items are allowed')
        # clear active items and set flag
        active_items, flag = [], True
        return 429
    elif not flag:
        active_items.append(item)
        return 200
    elif flag:
        print("Called again after cleared items")
        return 200

with a test that calls on this function several times, and with the expectation that the nth call will raise the 429 we're looking for - and then after x amount of time, that the function gets called again.

[denisov-vlad]

I still don't understand how to create such a test with backoff. When we run the run_query method, it will be restarted in the background, so we can't handle it.

By the way, added simple query test.

[guidopetri]

Alright, no worries. I can try giving it a shot this weekend or next.

[guidopetri]

@denisov-vlad shouldn't this be if r.status_code == 429? Can you provide an example 429 response from Metrica?

[denisov-vlad]

@guidopetri thanks! Yes, r.status_code is better here.

[guidopetri]

@denisov-vlad I converted the test to pytest and added a 429 test. The __call__ function is replacing requests.get(), essentially, so once we call it a 3rd time (according to N_API_CALLS) it returns a 429 / failing response, and then afterwards returns success; so we expect to call requests.get() N_API_CALLS + 1 times. Does this make sense? Do you think this test looks good, or do you think anything should be changed?

[denisov-vlad]

@guidopetri thank you! The updated code looks good.