-
Notifications
You must be signed in to change notification settings - Fork 0
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
Create hourly product in moorings products pipeline #206
Conversation
(and update required version of aodntools)
b6d04f9
to
ecebe8c
Compare
Codecov Report
@@ Coverage Diff @@
## master #206 +/- ##
=======================================
Coverage 88.84% 88.84%
=======================================
Files 44 44
Lines 2510 2510
Branches 420 420
=======================================
Hits 2230 2230
Misses 162 162
Partials 118 118 Continue to review full report at Codecov.
|
|
||
for qc_flags in self.product_qc_flags: | ||
|
||
product_url, errors = hourly_aggregator(input_list, self.product_site_code, qc_flags, self.temp_dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume product_url is different given the qc_flags
arguments!?
@@ -54,19 +54,32 @@ def test_good_manifest(self, mock_webfeatureservice): | |||
self.assertCountEqual(INPUT_FILE_COLLECTION.get_attribute_list('dest_path'), | |||
handler.input_file_collection.get_attribute_list('dest_path') | |||
) | |||
self.assertEqual(len(handler.file_collection), 5) | |||
|
|||
self.assertEqual(len(handler.file_collection), 7) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the 7 hardcoding? If it's because this class is only executed for a single test, them grab the number of file_collection as args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think the test_good_manifest is doing too much stuff - maybe some function refactoring !?
|
||
self.assertEqual(len(handler.file_collection), 7) | ||
|
||
# check new product files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part can be a test function
|
||
# check deletion of previous versions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part can also be a separate one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I leave some comments... The thing that poped out was that you can select nested list with QC flags to create different products but to know what will be the end result is hard.
Maybe a simple dictionary with names will help disambiguate at creation and when debugging.
Thanks @ocehugo . I have responded to your comments and made some changes. Please merge if you're happy with it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better - I left some replies given your changes so merge is up to you:)
Thanks for the comments @ocehugo . I might address some of them later, but I'd like to merge this now. |
No description provided.