open-metadata / OpenMetadata

OpenMetadata is a unified metadata platform for data discovery, data observability, and data governance powered by a central metadata repository, in-depth column level lineage, and seamless team collaboration.
https://open-metadata.org
Apache License 2.0
5.55k stars 1.05k forks source link

MINOR: DI APP Support Partial Success #18017

Open ulixius9 opened 1 month ago

ulixius9 commented 1 month ago

Describe your changes:

Fixes

I worked on ... because ...

#

Type of change:

#

Checklist:

github-actions[bot] commented 1 month ago

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR. You can also use pre-commit to automate the Java code formatting.

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

github-actions[bot] commented 1 month ago

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR. You can also use pre-commit to automate the Java code formatting.

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

ulixius9 commented 1 month ago

@sushi30 the primary goal of this status is to take care of Entity type table a0749875-d042-415b-accb-894ab6bc2977 does not have expected relationship contains to/from entity type databaseSchema type of error. Whenever application fails due to this error a user might think that there is something wrong with the application itself but that's not true right Application works fine but there is some issue with the asset itself. and also the main goal of this pr is also to make such errors ignore-able for AUTs, currently AUTs fails because our application fails with failed status, it will pass if there is a partial success or success status.

We have followed the same logic that we have for ingestion, i.e if more then 90% of the assets succeeded then the application will be marked as partial success.

sonarcloud[bot] commented 1 month ago

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

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

sushi30 commented 1 month ago

I do not see how this PR addresses this issue, unlike the ingestion, this app interacts only with internal components (and not external systems) so we should have a clear state for the app. If this requires the user's attention then we can log a warning, if it does not we can log at a lower level. I suggest making it clear to the user what is the action they should take when the app finishes (retry, reindex, re-ingest etc.). For this case it should be either success (the user can expect DI dashboards to be populated) or failure (look at the logs, resolve the issue and re-run).

For the AUTs I would suggest that we make the failure ignore-able at the test level. I do not think we should make schema changes (which are user-facing interfaces) to address issues with our testing frameworks.

Can we make the app not raise an error for specific types of API errors like the one you stated?

ulixius9 commented 1 month ago

I mean the search reindexing application marks the status directly as success in case of such error which IMO might be not apt but we can replicate the same here.