vdk-core: Improve ingestion error logging #930
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, after ingestion completes, a user is presented
with the following message:
```
Successful uploads:1
Failed uploads:0
ingesting plugin errors:defaultdict(<class 'vdk.internal.builtin_plugins.ingestion.ingester_utils.AtomicCounter'>, {})
```
This is not ideal, as printing internal Python representations
in user logs might be misleading, as a user might believe
some error has occured when none did.
This change fixes this by printing `None` instead when no
errors occur, and printing the dictionary result instead of
the defaultdict representation when errors did occur.
Testing done: tested locally
Signed-off-by: Gabriel Georgiev <gageorgiev@vmware.com>
Signed-off-by: Gabriel Georgiev <gageorgiev@vmware.com>
doks5
approved these changes
Aug 3, 2022
projects/vdk-core/src/vdk/internal/builtin_plugins/ingestion/ingester_base.py
Show resolved
Hide resolved
duyguHsnHsn
pushed a commit
that referenced
this pull request
Aug 4, 2022
Currently, after ingestion completes, a user is presented
with the following message:
```
Successful uploads:1
Failed uploads:0
ingesting plugin errors:defaultdict(<class 'vdk.internal.builtin_plugins.ingestion.ingester_utils.AtomicCounter'>, {})
```
This is not ideal, as printing internal Python representations
in user logs might be misleading, as a user might believe
some error has occured when none did.
This change fixes this by printing `None` instead when no
errors occur, and printing the dictionary result instead of
the defaultdict representation when errors did occur.
```
Successful uploads: 1
Failed uploads: 0
Ingesting plugin errors: None
```
Testing done: tested locally
Signed-off-by: Gabriel Georgiev <gageorgiev@vmware.com>
duyguHsnHsn
added a commit
that referenced
this pull request
Aug 8, 2022
* vdk-csv: add export-csv command * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * vdk-csv: modified requirements.txt * vdk-csv: modified requirements.txt * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * vdk-csv: changes on failing tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * vdk-csv: changes on failing tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * vdk-csv: changes on failing tests and adding examples * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * vdk-csv: changes on failing tests * cicd: increase memory limits for cicd runner (#927) what: We intend to increase the limits to our cicd containers.Updated values.yaml file. The script "install-runners.sh" has to be manually triggered for changes to take effect. why: Control Service Integration Tests have been failing with OOM errors. Although this doesn't directly update the values it servers as a reference point for future changes. testing: n/a Signed-off-by: Momchil Zhivkov mzhivkov@vmware.com * versatile-data-kit: make easier slack instructions (#925) Add a link to Slack inviter (https://communityinviter.com/apps/cloud-native/cncf) to make it easier for people to join CNCF slack. Otherwise, they need to read the docs and it becomes more complex. Signed-off-by: Antoni Ivanov <aivanov@vmware.com> * vdk-airflow: populate readme (#924) * control-service: increase integration test builder memory (#929) what: Increased the gradle builder/worker limits why: Integration tests keep failing with memory issues. testing: tests complete Signed-off-by: Momchil Zhivkov mzhivkov@vmware.com * [pre-commit.ci] pre-commit autoupdate (#928) updates: - [github.com/asottile/pyupgrade: v2.37.2 → v2.37.3](asottile/pyupgrade@v2.37.2...v2.37.3) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Andy <87700128+doks5@users.noreply.github.com> * vdk-core: add memory properties client (#921) * vdk-core: add memory properties client In case Control Service is not available, for demo purposes, it would be good to be able to use some simple client so that jobs that stil use properties would work. To make it clear that it's not recommended for production (after all properties are supposed to be used to store state, so in memory client is very pointless) we write warning each time write proerties is called. Signed-off-by: Antoni Ivanov <aivanov@vmware.com> * vdk-examples: add ingest and anonymize example (#922) * vdk-examples: add ingest and anonymize example The example was inspired by some talks with potential users. But regardless it's pretty interesting and useful example and added it to the collection of examples we have. We do not have a lot of examples of how plugins can be used and this shows they can be pretty powerful. Signed-off-by: Antoni Ivanov <aivanov@vmware.com> * vdk-core: Improve ingestion error logging (#930) Currently, after ingestion completes, a user is presented with the following message: ``` Successful uploads:1 Failed uploads:0 ingesting plugin errors:defaultdict(<class 'vdk.internal.builtin_plugins.ingestion.ingester_utils.AtomicCounter'>, {}) ``` This is not ideal, as printing internal Python representations in user logs might be misleading, as a user might believe some error has occured when none did. This change fixes this by printing `None` instead when no errors occur, and printing the dictionary result instead of the defaultdict representation when errors did occur. ``` Successful uploads: 1 Failed uploads: 0 Ingesting plugin errors: None ``` Testing done: tested locally Signed-off-by: Gabriel Georgiev <gageorgiev@vmware.com> * vdk-core,vdk-impala,vdk-lineage,vdk-trino: Support for pluggy 1.0 (#931) The 1.0 release of `pluggy` introduced a breaking change by renaming its `callers` module to `_callers`. This has been fixed by using the `HookCallResult` constant from `vdk.api.plugin.plugin_registry` instead of `pluggy.callers._Result` everywhere necessary, and amending said constant to be either `pluggy.callers._Result` or `pluggy._callers._Result` dynamically based on the version of `pluggy` in the current Python env. Also fixed a test which had `hookwrapper` set to True for some reason inside its testing plugin. Testing done: ran tests locally, CICD Signed-off-by: Gabriel Georgiev <gageorgiev@vmware.com> * control-service: Atomic job cancellation (#860) Currently, there is a possibility for a job to be set to be cancelled, and have it complete before the cancellation can complete. This leads to a 500 error with no explanation. This change introduces an additional check that the operation response or its status is not null, and if it is, it logs an appropriate message and raises an appropriate exception. Testing done: unit tests Signed-off-by: Gabriel Georgiev <gageorgiev@vmware.com> Co-authored-by: Miroslav Ivanov <miroslavi@vmware.com> Co-authored-by: Momchil Z <mzhivkov@vmware.com> * vdk-csv: changes on unit tests * vdk-csv: changes on no data in database handling and new unit tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * vdk-impala, vdk-trino: Remove deprecated use of result field (#933) The _Result object from pluggy features a result field, which was deprecated in earlier versions in favour of get_result(), and removed entirely in 1.0. This change removes references to it which caused an error for a heartbeat test. Testing done: CICD Signed-off-by: Gabriel Georgiev <gageorgiev@vmware.com> * vdk-csv: add helper methods and delete unused imports * vdk-csv: add helper methods and delete unused imports * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Momchil Z <mzhivkov@vmware.com> Co-authored-by: Antoni Ivanov <aivanov@vmware.com> Co-authored-by: Andy <87700128+doks5@users.noreply.github.com> Co-authored-by: Gabriel Georgiev <45939426+gageorgiev@users.noreply.github.com> Co-authored-by: Miroslav Ivanov <miroslavi@vmware.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, after ingestion completes, a user is presented
with the following message:
This is not ideal, as printing internal Python representations
in user logs might be misleading, as a user might believe
some error has occured when none did.
This change fixes this by printing
Noneinstead when noerrors occur, and printing the dictionary result instead of
the defaultdict representation when errors did occur.
Testing done: tested locally
Signed-off-by: Gabriel Georgiev gageorgiev@vmware.com