Run granules sequentially in same working dir #26

adybbroe · 2023-05-22T09:33:39Z

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

…ctory was not created Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

…by using get and checking for None messages at time out of the for loop Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

…ssages Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

…data handling in one function for now Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

…b.com:adybbroe/pytroll-cspp-runner into run-granules-sequentially-in-same-working-dir

gerritholl · 2023-05-22T15:18:41Z

cspp_runner/tests/conftest.py

+# Copyright (c) 2023 Adam.Dybbroe
+
+# Author(s):
+
+#   Adam.Dybbroe <a000680@c21856.ad.smhi.se>


This should be pytroll-cssp-runner developers, and then the names of the authors can/should be in a separate AUTHORS file.

gerritholl

Lots of good work here. I see you continued refactoring and improved the overall code base. However, that makes it difficult to see what is actually being changed. My comments are on the details, but I'm missing the overall picture. It's also a bit tricky to see what code was removed and what code was moved + changed.

Can you please document the PR? I mean the PR description here on GitHub, which is currently empty. The title does not to justice to the refactoring/rewriting carried out in this PR. It would be helpful if you could edit the title and description of the PR and summarise functional changes, refactoring changes, and any possible backward-compatibility changes.

gerritholl · 2023-05-22T15:19:20Z

cspp_runner/cspp_utils.py

+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+


These lines are not needed.

gerritholl · 2023-05-22T15:20:12Z

cspp_runner/cspp_utils.py

+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+# Copyright (c) 2023 Pytroll Developers


Suggested change

# Copyright (c) 2023 Pytroll Developers

# Copyright (c) 2023 pytroll-cspp-runner Developers

gerritholl · 2023-05-22T15:20:46Z

cspp_runner/cspp_utils.py

+
+_NPP_SDRPROC_LOG_FILE = os.environ.get('NPP_SDRPROC_LOG_FILE', None)
+
+LOG = logging.getLogger(__name__)


Not a constant, so logger would be a better choice as a variable name.

gerritholl · 2023-05-22T15:21:12Z

cspp_runner/cspp_utils.py

+LOG = logging.getLogger(__name__)
+
+
+class CSPPAncillaryDataUpdater():


Suggested change

class CSPPAncillaryDataUpdater():

class CSPPAncillaryDataUpdater:

gerritholl · 2023-05-22T15:26:01Z

cspp_runner/cspp_utils.py

+    def initalize_working_dir(self):
+        """Check and set the CSPP working directory."""
+        _check_environment("CSPP_WORKDIR")
+        self.cspp_workdir = os.environ.get("CSPP_WORKDIR", '')


Suggested change

self.cspp_workdir = os.environ.get("CSPP_WORKDIR", '')

self.cspp_workdir = os.environ["CSPP_WORKDIR"]

you're already enforcing its existence in the line above, so no need to guard and provide a default value that would anyway lead to failure in the following line

gerritholl · 2023-05-22T15:57:13Z

cspp_runner/runner.py

+            # self.pool.apply_async(self.spawn_cspp, [keeper],
+            #                       {"publisher": publisher,
+            #                        "viirs_sdr_call": viirs_sdr_call,
+            #                        "viirs_sdr_options": viirs_sdr_options,
+            #                        "granule_time_tolerance": granule_time_tolerance
+            #                        }))


Suggested change

# self.pool.apply_async(self.spawn_cspp, [keeper],

# {"publisher": publisher,

# "viirs_sdr_call": viirs_sdr_call,

# "viirs_sdr_options": viirs_sdr_options,

# "granule_time_tolerance": granule_time_tolerance

# }))

gerritholl · 2023-05-22T16:00:03Z

cspp_runner/runner.py


    ncpus_available = multiprocessing.cpu_count()
    LOG.info("Number of CPUs available = " + str(ncpus_available))
-    LOG.info("Will use %d CPUs when running CSPP instances" % ncpus)
-    viirs_proc = ViirsSdrProcessor(ncpus, level1_home)
+    LOG.info("Will use %s CPUs when running CSPP instances" % str(ncpus))


Detail, but why this change? Are we not sure ncpus is an int?

gerritholl · 2023-05-22T16:01:05Z

cspp_runner/tests/conftest.py

+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+


This part is redundant.

gerritholl · 2023-05-22T16:01:38Z

cspp_runner/tests/conftest.py

+# @pytest.fixture
+# def fake_empty_viirs_sdr_files_nine_seconds_deviation(tmp_path):
+#     """Write fake empty viirs sdr files."""
+#     starttime = datetime.datetime(2023, 5, 10, 14, 30, 53, 800000) + datetime.timedelta(seconds=9)
+#     return create_full_sdr_list_from_time(tmp_path, starttime)
+
+


Suggested change

# @pytest.fixture

# def fake_empty_viirs_sdr_files_nine_seconds_deviation(tmp_path):

# """Write fake empty viirs sdr files."""

# starttime = datetime.datetime(2023, 5, 10, 14, 30, 53, 800000) + datetime.timedelta(seconds=9)

# return create_full_sdr_list_from_time(tmp_path, starttime)

gerritholl · 2023-05-22T16:06:32Z

setup.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-

-# Copyright (c) 2013 - 2022 pytroll-cspp-runner developers
+# Copyright (c) 2013 - 2023 Pytroll Developers


Should be pytroll-cspp-runner still?

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

codecov · 2023-05-30T09:24:38Z

Codecov Report

Merging #26 (9de13f7) into main (6f22cca) will increase coverage by 3.17%.
The diff coverage is 95.20%.

❗ Current head 9de13f7 differs from pull request most recent head 25f75d2. Consider uploading reports for the commit 25f75d2 to get more accurate results

@@            Coverage Diff             @@
##             main      #26      +/-   ##
==========================================
+ Coverage   78.59%   81.77%   +3.17%     
==========================================
  Files          12       14       +2     
  Lines         995     1196     +201     
==========================================
+ Hits          782      978     +196     
- Misses        213      218       +5

Flag	Coverage Δ
unittests	`81.77% <95.20%> (+3.17%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cspp_runner/tests/test_viirs_dr_runner.py	`100.00% <ø> (ø)`
cspp_runner/viirs_dr_runner.py	`96.61% <ø> (-0.06%)`	⬇️
cspp_runner/runner.py	`85.66% <88.18%> (+0.68%)`	⬆️
cspp_runner/cspp_utils.py	`93.93% <93.93%> (ø)`
cspp_runner/tests/test_runner.py	`98.00% <95.89%> (-0.98%)`	⬇️
cspp_runner/post_cspp.py	`69.76% <96.42%> (+1.16%)`	⬆️
cspp_runner/tests/conftest.py	`100.00% <100.00%> (ø)`
cspp_runner/tests/test_post_cspp.py	`100.00% <100.00%> (ø)`
cspp_runner/tests/test_utils.py	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Adam.Dybbroe added 8 commits May 5, 2023 11:11

Try fix so sdr files get moved and published when ready

5f7c394

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Simplifying the viirs runner and skip the 3-granule setup all together

9fd6f28

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Bugfix, apply_async requires the arguments as a list and working dire…

8340293

…ctory was not created Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Improved checking that the number of SDR result files are okay

51f3f89

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Try fixing the beahviour after end of a pass - "freeing" the threads …

bc7e6f2

…by using get and checking for None messages at time out of the for loop Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Clean up and slight re-arrange of data flow and improvement on log me…

290a3e9

…ssages Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Refatoring functions back to methods. And collect the lut/anciallary …

b5e9e1d

…data handling in one function for now Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Refactor the LUT/Ancillary data updating

9fa4c3e

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

adybbroe requested a review from gerritholl May 22, 2023 09:33

adybbroe and others added 3 commits May 22, 2023 11:38

Merge branch 'main' into run-granules-sequentially-in-same-working-dir

9d64ef8

Add freezegun to the ci env

8a87323

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Merge branch 'run-granules-sequentially-in-same-working-dir' of githu…

343f31d

…b.com:adybbroe/pytroll-cspp-runner into run-granules-sequentially-in-same-working-dir

adybbroe marked this pull request as ready for review May 22, 2023 14:25

gerritholl reviewed May 22, 2023

View reviewed changes

Improved handling setting necessary environment affecting lut updating

7588d15

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Adam.Dybbroe added 5 commits May 30, 2023 12:32

Bugfix - missing lut's dir setting

9de13f7

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Make it possible to run CSPP in specific granule specific working-dirs

46e2a35

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Bugfix - working dir should be a pathlib path

a62006c

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Bug fixes, and clean working dir after end of processing

4d28637

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Skip hostname and scheme in uri in output message

25f75d2

Signed-off-by: Adam.Dybbroe <a000680@c21856.ad.smhi.se>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run granules sequentially in same working dir #26

Run granules sequentially in same working dir #26

adybbroe commented May 22, 2023 •

edited

Loading

gerritholl May 22, 2023

gerritholl left a comment

gerritholl May 22, 2023

gerritholl May 22, 2023

gerritholl May 22, 2023

gerritholl May 22, 2023

gerritholl May 22, 2023

gerritholl May 22, 2023

gerritholl May 22, 2023

gerritholl May 22, 2023

gerritholl May 22, 2023

gerritholl May 22, 2023

codecov bot commented May 30, 2023 •

edited

Loading

	# Copyright (c) 2023 Pytroll Developers
	# Copyright (c) 2023 pytroll-cspp-runner Developers


		_NPP_SDRPROC_LOG_FILE = os.environ.get('NPP_SDRPROC_LOG_FILE', None)

		LOG = logging.getLogger(__name__)

		LOG = logging.getLogger(__name__)


		class CSPPAncillaryDataUpdater():

	class CSPPAncillaryDataUpdater():
	class CSPPAncillaryDataUpdater:

	self.cspp_workdir = os.environ.get("CSPP_WORKDIR", '')
	self.cspp_workdir = os.environ["CSPP_WORKDIR"]

Run granules sequentially in same working dir #26

Are you sure you want to change the base?

Run granules sequentially in same working dir #26

Conversation

adybbroe commented May 22, 2023 • edited Loading

Choose a reason for hiding this comment

gerritholl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 30, 2023 • edited Loading

Codecov Report

adybbroe commented May 22, 2023 •

edited

Loading

codecov bot commented May 30, 2023 •

edited

Loading