This repository was archived by the owner on Nov 30, 2022. It is now read-only.
B/add continuous background job handler #757
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
Alexander-Dubrawski
requested review from
caterinamandel98,
cH3n7i,
fabianhe,
kathwill,
MaxSchneider1337,
monasobh,
PeterTsayun and
schTi
as code owners
Alexander-Dubrawski
requested review from
Bensk1 and
Bouncner
and removed request for
cH3n7i,
MaxSchneider1337,
caterinamandel98,
fabianhe,
PeterTsayun,
schTi,
monasobh and
kathwill
Bensk1
suggested changes
Oct 1, 2020
Comment on lines
+42
to
+56
| assert continuous_job_handler._connection_factory == "connection_factory" | ||
| assert continuous_job_handler._hyrise_active == "hyrise_active" | ||
| assert continuous_job_handler._worker_pool == "worker_pool" | ||
| assert ( | ||
| continuous_job_handler._storage_connection_factory | ||
| == "storage_connection_factory" | ||
| ) | ||
| assert continuous_job_handler._previous_system_data == { | ||
| "previous_system_usage": None, | ||
| "previous_process_usage": None, | ||
| } | ||
| assert continuous_job_handler._previous_chunk_data == { | ||
| "value": None, | ||
| } | ||
| assert continuous_job_handler._scheduler == mock_background_scheduler_obj |
There was a problem hiding this comment.
Are these asserts the correct way for testing? If the asserts fail, the program crashes, doesn't it? I expected something like assertEqual
There was a problem hiding this comment.
This is the normal syntax for pytest. pytest is a third-party unit test framework with a more lighter-weight syntax. If an assert fails only the test fails.
…hub.com/hyrise/Cockpit into B/Add_continuous_background_job_handler
Bensk1
approved these changes
Oct 6, 2020
Alexander-Dubrawski
deleted the
B/Add_continuous_background_job_handler
branch
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR is the secound of four which aims to refactor the handling of Hyrise jobs (it aims to split #664). Hyrise jobs are interacting directly with the Hyrise to get the meta information and managing the Hyrise settings. We have three kinds of jobs:
Both continuous and asynchronous jobs are executed via the apscheduler (https://apscheduler.readthedocs.io/en/stable/). Esspacillay with the asynchronous jobs that's not necessary and increases the complexity (we could just use normal python threads). All three kinds of jobs are scattered in the database and background_scheduler component. The goal of the refactoring is now to have a clear separation for the jobs in the three categories (continuous, asynchronous, and synchronous) and define handlers for that. Moreover, we can stop the continuous jobs while loading/deleting tables (hyrise/hyrise#2180).
This PR implements the handler for continuous jobs.
ToDo