-
Notifications
You must be signed in to change notification settings - Fork 57
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
vdk-csv: add export-csv command #934
Merged
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
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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 [email protected]
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 <[email protected]>
what: Increased the gradle builder/worker limits why: Integration tests keep failing with memory issues. testing: tests complete Signed-off-by: Momchil Zhivkov [email protected]
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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]> Co-authored-by: Miroslav Ivanov <[email protected]> Co-authored-by: Momchil Z <[email protected]>
for more information, see https://pre-commit.ci
Something with this pull request seems odd. There seem to be changes that have already been merged? |
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 <[email protected]>
for more information, see https://pre-commit.ci
antoniivanov
approved these changes
Aug 8, 2022
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.
vdk-csv: add export-csv command
What: Extended vdk-csv by adding a new command called vdk export-csv which can export from the default database to a local file.
Why: Often users would need to export result of some query to CSV file so they can send it to a colleague or to analyse it with other excel files they already have or to import it somewhere else.
Testing Done: added an unit test
Signed-off-by: Duygu Hasan [email protected]