Skydio / revup

Effortlessly create and manage pull requests without changing branches. Powers a stacked diffs workflow with python and git "plumbing" commands.
https://github.com/Skydio/revup
MIT License
317 stars 67 forks source link

Fail on bad response status #82

Closed aaron-skydio closed 1 year ago

aaron-skydio commented 2 years ago

Previously we could get responses from the github api like:

Response status: 401 took 0.21685314178466797
Response JSON:
{
 "message": "Bad credentials",
 "documentation_url": "https://docs.github.com/graphql"
}

But continue on and fail with something like:

Traceback (most recent call last):
  File "/.../lib/python3.8/site-packages/revup/__main__.py", line 17, in _main
    sys.exit(asyncio.run(main()))
  File "/usr/lib/python3.8/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/.../lib/python3.8/site-packages/revup/revup.py", line 357, in main
    return await upload.main(
  File "/.../lib/python3.8/site-packages/revup/upload.py", line 46, in main
    await topics.query_github()
  File "/.../lib/python3.8/site-packages/revup/topic_stack.py", line 961, in query_github
    ) = await github_utils.query_everything(
  File "/.../lib/python3.8/site-packages/revup/github_utils.py", line 290, in query_everything
    this_node = pr_result["data"]["repository"][prs_out[i]]
KeyError: 'data'

Now you get an error like:

E: Request failed with response status 401
E: Response: {'message': 'Bad credentials', 'documentation_url': 'https://docs.github.com/graphql'}

Not sure if we should treat any 200 status as ok or something, or just 400 statuses as errors, etc.

Reviewers: jerry,brian-k Topic: status-fail

aaron-skydio commented 2 years ago

Reviews in this chain: └https://github.com/Skydio/revup/pull/82 Fail on bad response status

aaron-skydio commented 2 years ago
# head base diff date summary
0 74069b34 311ff10d diff Oct 28 15:30 PM 3 files changed, 16 insertions(+), 1 deletion(-)
aaron-skydio commented 2 years ago

Also tested this works correctly on successful requests (e.g. to upload this PR :) )