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.

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

Jump to bottom

MINOR - Better handling of Ingestion Pipeline Status #14792

Merged
merged 3 commits into from
Jan 23, 2024
Merged

MINOR - Better handling of Ingestion Pipeline Status #14792

merged 3 commits into from
Jan 23, 2024

Conversation

pmbrull
Copy link
Collaborator

@pmbrull pmbrull commented Jan 19, 2024

Describe your changes:

Setting the wrong service name in the ingestionPipelineFQN values in the YAML hangeg the workflow, since it failed before stopping the timer.

Adding a couple of things here:

  1. Since we are creating the Ingestion Pipeline already, using its FQN. This means that even if users set the wrong serviceName in the ingestionPipelineFQN, it will still work. I am not changing this to only pipeline name or anything since it would require to patch the pipeline service client logic.
  2. Adding exception handling so that if something explodes, the workflow gets stopped properly

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.

@pmbrull pmbrull requested a review from a team as a code owner January 19, 2024 09:38
@github-actions github-actions bot added Ingestion safe to test Add this label to run secure Github workflows on PRs labels Jan 19, 2024
@github-actions GitHub Actions
Copy link
Contributor

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions GitHub Actions
Copy link
Contributor

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@sonarqubecloud SonarQubeCloud
Copy link

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

Kudos, no new issues were introduced!

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

See analysis details on SonarCloud

IceS2
IceS2 approved these changes Jan 23, 2024
View reviewed changes
Copy link
Contributor

@IceS2 IceS2 left a comment

Choose a reason for hiding this comment

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

LGTM.

Since I don't have enough context yet I'm unsure but I guess the Source import was due to some copy paste since this Workflow seems a bit special and without the need of a source at all, right?

Thanks (=

@pmbrull
Copy link
Collaborator Author

pmbrull commented Jan 23, 2024

LGTM.

Since I don't have enough context yet I'm unsure but I guess the Source import was due to some copy paste since this Workflow seems a bit special and without the need of a source at all, right?

Thanks (=

it was indeed some old copy paste :)

@pmbrull pmbrull merged commit 8f2c2b2 into open-metadata:main Jan 23, 2024
12 checks passed
@pmbrull pmbrull deleted the ingest-fqn-err branch January 23, 2024 08:24
harshach pushed a commit that referenced this pull request Jan 24, 2024
@pmbrull @harshach
MINOR - Better handling of Ingestion Pipeline Status (#14792)
d8555fc 
* MINOR - Better handling of Ingestion Pipeline Status

* format

* format
Abhishek332 pushed a commit to Abhishek332/OpenMetadata that referenced this pull request Jan 25, 2024
@pmbrull @Abhishek332
MINOR - Better handling of Ingestion Pipeline Status (open-metadata#1…
3bdb120 
…4792)

* MINOR - Better handling of Ingestion Pipeline Status

* format

* format
harshach added a commit that referenced this pull request Feb 1, 2024
@harshach @pmbrull
@ulixius9 @ayush-shah @chirag-madlani @kwgdaig @TeddyCr @OnkarVO7 @Ashish8689 @Abhishek332 @luzlab @karanh37
Fix #14786: Suggestions API (#14821)
2e95fcb 
* Fix #14786: Suggestions API

* Handle suggestions in ometa

* Minor: Optimise Databricks Client (#14776)

* MINOR - Fix SP topology context & Looker usage context (#14816)

* MINOR - Fix SP topology context & Looker usage context

* MINOR - Fix SP topology context & Looker usage context

* Fix tests

* Fixes #14598: Fix Tags / Labels ingestion on includeTags as False (#14782)

* fix(ui): password error message for char limits (#14808)

* fix(ui): password error message for char limits

* fix java side code

* Fixes #13556: Support for Salesforce table description ingestion (#14733)

* ISSUE-13556: Add suport for Salesforce table description ingestion

* ISSUE-13556: Remove unnecessary blank line

* ISSUE-13556: Fix to get description for each table

---------

Co-authored-by: Teddy <teddy.crepineau@gmail.com>

* MINOR - Better handling of Ingestion Pipeline Status (#14792)

* MINOR - Better handling of Ingestion Pipeline Status

* format

* format

* MINOR: Added table validation for cost analysis data (#14793)

* Added validation for cost analysis source

* centralized life cycle logic

* CYPRESS: simplify side navigation click in cypress (#14818)

* simplify side navigation click in cypress

* make sidbar item uses common enum

* fix cypress failure of outside import

* fix(#14326): tier dropdown is not working in advance search (#14780)

* improvement in advance search based on custom property

* fix a reading undefined property issue

* wip: advance search based on tier

* some code cleanup and improvement

* some fixes

* fix: ui flicker when advanceSearched is apply and refresh the page

* some cleanup

* no need to call customproperty api call, if entity not suppport customProperties

* minor change

* fix: autocomplete not working in tier search option in advance search modal

* added unit test for advance search provider component

* some cleanup

* added testcase for open modal

* added testcase for resetAllFilters method

* removed unwanted code

* added e2e test for testing tier advance search

* fix: e2e search flow for single field

* fix: string field not working after giving listValues in TierSearch

* fix: group query e2e test fix

* used asyncFetch way to get the tierOptions synchronously

* some cleanup

* remove unwanted lines

* some cleanup

* fix: selected option show option value instead of option title

* fix(minor): update skip icon for executions (#14809)

* Fixes #14803: ignore capitalization when confirming deletes  (#14804)

* ignore case when confirming deletes

* Test confirmation of deletes works when case differs 

Added test case for 'delete' as the confirmation text.

* minor(config): update openmetadata-ui code reviewers (#14823)

* Add Tests

* Add list/accept/reject apis

* initial ui changes

* localisation

* show suggestion for empty description

* ui feedbacks

* Fix permission check for entities without owner

* Fix entityLink and add tests

* Add update suggestion WIP

* Fix test

* Fix PUT and Pagination

* Fix styling

* update test

* Update status

* add OM server connection in apps

* add permissions check

* Fix CI

* Remove TODO

* Fix feedResourceTest

* fix unit tests

* add private configs for apps

* add private configs for apps

* fix update application icons

* minor center align icon

* add private configs for apps

* Format

* Fix pydantic gen

* Remove token

* Update name

* Rework private conf

* Fix apps

* Fix apps

* Format

* Format

* show metapilot only if its installed

---------

Co-authored-by: Pere Miquel Brull <peremiquelbrull@gmail.com>
Co-authored-by: Mayur Singal <39544459+ulixius9@users.noreply.github.com>
Co-authored-by: Ayush Shah <ayush@getcollate.io>
Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com>
Co-authored-by: kwgdaig <18678754+kwgdaig@users.noreply.github.com>
Co-authored-by: Teddy <teddy.crepineau@gmail.com>
Co-authored-by: Onkar Ravgan <onkar.10r@gmail.com>
Co-authored-by: Ashish Gupta <ashish@getcollate.io>
Co-authored-by: Abhishek Porwal <80886271+Abhishek332@users.noreply.github.com>
Co-authored-by: Carlo Q <carlo@machina.bio>
Co-authored-by: karanh37 <karanh37@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayush-shah ayush-shah ayush-shah approved these changes

@IceS2 IceS2 IceS2 approved these changes

Assignees
No one assigned
Labels
Ingestion safe to test Add this label to run secure Github workflows on PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pmbrull @IceS2 @ayush-shah