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 13823: Validate and Parse gracefully IngestionPipeline #14461

Merged
merged 3 commits into from
Dec 22, 2023

Conversation

OnkarVO7
Copy link
Contributor

Describe your changes:

Fixes #13823

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

@OnkarVO7 OnkarVO7 requested a review from a team as a code owner December 20, 2023 14:19
@github-actions github-actions bot added Ingestion safe to test Add this label to run secure Github workflows on PRs labels Dec 20, 2023
ingestion_pipeline = IngestionPipeline.parse_obj(config_dict)
return ingestion_pipeline

except ValidationError:
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we take care of the serviceConnection? Not only the sourceConfig

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe that's where we get also a lot of noise, for example when we configure an URI without the scheme or something

Copy link
Collaborator

Choose a reason for hiding this comment

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

as discussed, that was the wrong comment. We'll be adding it to the run_automations endpoint though

@@ -400,37 +406,84 @@ def parse_workflow_config_gracefully(
raise ParsingConfigurationError("Uncaught error when parsing the workflow!")


def parse_test_connection_request_gracefully(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the method since it wasn't being used anywhere except for tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks

@OnkarVO7 OnkarVO7 requested a review from pmbrull December 21, 2023 16:00
Copy link

Quality Gate Passed Quality Gate passed for 'open-metadata-airflow-apis'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

Quality Gate Passed Quality Gate passed for 'open-metadata-ingestion'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
75.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@OnkarVO7 OnkarVO7 merged commit 79444f4 into main Dec 22, 2023
18 of 19 checks passed
@OnkarVO7 OnkarVO7 deleted the ingestion_pipeline_parsing branch December 22, 2023 04:26
OnkarVO7 added a commit that referenced this pull request Dec 22, 2023
* fixed ingestion pipeline parsing

* Added validation for automation workflow

---------

Co-authored-by: Pere Miquel Brull <peremiquelbrull@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ingestion safe to test Add this label to run secure Github workflows on PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ingestion Pipeline pydantic validations w/ airflow apis
2 participants