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

fix(glue): fix typo in reported warning, report with flow_urn #8138

Merged
merged 5 commits into from
Jun 12, 2023

Conversation

mayurinehate
Copy link
Collaborator

Log warnings and failures. We lose some warnings/failures due to Lossy nature of report.failures and report.warnings, in case there are more than 10 elements.

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@github-actions github-actions bot added the ingestion PR or Issue related to the ingestion of metadata label May 26, 2023
def get_dataflow_graph(self, script_path: str) -> Optional[Dict[str, Any]]:
def get_dataflow_graph(
self, script_path: str, flow_urn: str
) -> Optional[Dict[str, Any]]:
"""
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tracking is easier with key flow_urn. script_path is already present in reported message.

self.report.report_failure(
script_path,
self.report_warning(
flow_urn,
f"Unable to download DAG for Glue job from {script_path}, so job subtasks and lineage will be missing: {e}",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the glue script can not be accessed due to some reason (missing permission - 403 Or missing script - 404), treat this as warning than error.

@asikowitz asikowitz merged commit f3cf9b7 into datahub-project:master Jun 12, 2023
42 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ingestion PR or Issue related to the ingestion of metadata
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants