Skip to content

fix(ingest): raise errors in more cases in rest sink #2638

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.

Sign up for GitHub

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

Closed
gabe-lyons wants to merge 3 commits into from
Closed

fix(ingest): raise errors in more cases in rest sink #2638

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd6b7be
raise errors in more cases rest sink
gabe-lyons Jun 3, 2021
0e38b55
more intentionality with raising errors
gabe-lyons Jun 3, 2021
33b6164
lint'
gabe-lyons Jun 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
more intentionality with raising errors
  • Loading branch information
@gabe-lyons
gabe-lyons committed Jun 3, 2021
commit 0e38b553a145871f8cf0076d86c061d58e2d8a2e
10 changes: 7 additions & 3 deletions metadata-ingestion/src/datahub/emitter/rest_emitter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,17 @@ def emit_mce(self, mce: MetadataChangeEvent) -> None:

response.raise_for_status()
except HTTPError as e:
response.raise_for_status()
info = response.json()
try:
info = response.json()
except Exception as parse_exception:
response.raise_for_status()
raise OperationalError(
"Unable to parse response from ingestion sink", {"message": str(parse_exception)}
) from e
Copy link
Contributor

@hsheth2 hsheth2 Jun 3, 2021

Choose a reason for hiding this comment

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

Since we're in except HTTPError as e:, we know that response.raise_for_status() will certainly throw here - probably want to remove that line since we already have it in e

We're actually not interested in the parse_exception since it just means that the response body wasn't json. Instead, we should set message to the response.text itself

raise OperationalError(
"Unable to emit metadata to DataHub GMS", info
) from e
except RequestException as e:
response.raise_for_status()
raise OperationalError(
"Unable to emit metadata to DataHub GMS", {"message": str(e)}
) from e