Add Celery job processing, routes, & BatchAggregation lib #783
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
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
There was a problem hiding this comment.
Some files could not be reviewed due to errors:
./panoptes_aggregation/batch_aggregation.py:11: [F401] 'panoptes_client.panop...
./panoptes_aggregation/batch_aggregation.py:11: [F401] 'panoptes_client.panoptes.PanoptesAPIException' imported but unused ./panoptes_aggregation/batch_aggregation.py:20: [E302] expected 2 blank lines, found 1 ./panoptes_aggregation/batch_aggregation.py:23: [F841] local variable 'exports' is assigned to but never used ./panoptes_aggregation/batch_aggregation.py:24: [F841] local variable 'wf_df' is assigned to but never used ./panoptes_aggregation/batch_aggregation.py:31: [F841] local variable 'reduced_data' is assigned to but never used ./panoptes_aggregation/batch_aggregation.py:33: [E302] expected 2 blank lines, found 1
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:2: [...
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:2: [F401] 'panoptes_aggregation.batch_aggregation.run_aggregation' imported but unused
CKrawczyk
reviewed
Mar 13, 2024
Comment on lines
6
to
8
| def test_save_exports(self): | ||
| # Test that Panoptes calls are made and files are saved | ||
| assert 1 == 1 |
There was a problem hiding this comment.
Will likely want to patch/mock the Paonoptes class and check the expected export calls are made when the function is run.
Comment on lines
10
to
15
| def test_process_wf_export(self): | ||
| # Test that: | ||
| # the wf export is parsed | ||
| # the version instance vars are set | ||
| # dataframe is retuned | ||
| assert 1 == 1 |
There was a problem hiding this comment.
A patch can be used to set a fixed input file stream and the test can check that it was parsed as expected. This can either be defined in the test file (similar to the extractor and reducer tests), or an example .csv file can be placed in the test folder and read in (might be easier to debug and read in the long run).
There was a problem hiding this comment.
Some files could not be reviewed due to errors:
./panoptes_aggregation/batch_aggregation.py:11: [F401] 'panoptes_client.panop...
./panoptes_aggregation/batch_aggregation.py:11: [F401] 'panoptes_client.panoptes.PanoptesAPIException' imported but unused ./panoptes_aggregation/batch_aggregation.py:20: [E302] expected 2 blank lines, found 1 ./panoptes_aggregation/batch_aggregation.py:23: [F841] local variable 'exports' is assigned to but never used ./panoptes_aggregation/batch_aggregation.py:24: [F841] local variable 'wf_df' is assigned to but never used ./panoptes_aggregation/batch_aggregation.py:31: [F841] local variable 'reduced_data' is assigned to but never used ./panoptes_aggregation/batch_aggregation.py:33: [E302] expected 2 blank lines, found 1
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:2: [...
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:2: [F401] 'unittest.mock.Mock' imported but unused ./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:7: [E302] expected 2 blank lines, found 1 ./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:28: [E201] whitespace after '[' ./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:28: [E202] whitespace before ']' ./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:28: [E202] whitespace before '}'
There was a problem hiding this comment.
Some files could not be reviewed due to errors:
./panoptes_aggregation/batch_aggregation.py:22: [E302] expected 2 blank lines...
./panoptes_aggregation/batch_aggregation.py:22: [E302] expected 2 blank lines, found 1 ./panoptes_aggregation/batch_aggregation.py:27: [F841] local variable 'wf_df' is assigned to but never used ./panoptes_aggregation/batch_aggregation.py:54: [E302] expected 2 blank lines, found 1
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:2: [...
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:2: [F401] 'unittest.mock.Mock' imported but unused
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:10: [E302] expected 2 blank lines, found 1
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:19: [E201] whitespace after '{'
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:19: [E202] whitespace before '}'
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:52: [E201] whitespace after '['
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:52: [E202] whitespace before ']'
./panoptes_aggregation/tests/batch_aggregation/test_batch_aggregation.py:52: [E202] whitespace before '}'
Merged
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.
This PR adds Celery/Redis based background job processing to aggregation. It includes routes to begin aggregation when a request is made from Panoptes directly and their specs. See cards on https://github.com/orgs/zooniverse/projects/44/ for details.
It also adds a BatchAggregation lib that interacts with Panoptes to begin to manage the full workflow. This code works when dropped into a console and as a background Celery job, but has no tests yet.
I'd love some feedback on what's here so far. I'm gonna message you directly, @CKrawczyk, and I'm opening this as a draft PR pointed at a branch off of master so that it can be tested in isolation. Also planning on a separate deploy to ensure things work as expected but I removed it from this PR for ease of viewing.