283 weekly roundup

[alexdunnjpl]

🗒️ Summary

Adds a module/script for extracting recently-updated/submitted DOIs from the local database and sending an email notification to the user specified in config['OTHER']['emailer_receivers']. The email is comprised of an html summary, and a json attachment for the same records.

⚙️ Test Data and/or Report

Unit tests pass. Comprehensive unit tests added for the new functionality.

♻️ Related Issues

• fixes #283

[alexdunnjpl]

Two sets of errors in automated tests, besides linting:

FAILED src/pds_doi_service/api/test/test_dois_controller.py::TestDoisController::test_post_dois_invalid_requests
FAILED src/pds_doi_service/api/test/test_dois_controller.py::TestDoisController::test_post_dois_reserve
FAILED src/pds_doi_service/api/test/test_dois_controller.py::TestDoisController::test_post_dois_update_w_payload
FAILED src/pds_doi_service/api/test/test_dois_controller.py::TestDoisController::test_post_dois_update_w_url

Those are (I think) the result of not having the following config values properly populated, though that surprises me given that I didn't notice those tests fail in #349 when I was looking at it earlier today:

[API_AUTHENTICATION]
# Add the issuer of the oauth tokens, for cognito https://cognito-idp.<aws-region>.amazonaws.com/<userpoolID>
jwt_issuer =
# Add the entire content of the JSON file at https://cognito-idp.<aws-region>.amazonaws.com/<userpoolID>/.well-known/jwks.json
json_web_key_set =
jwt_lifetime_seconds = 3600
jwt_algorithm = RS256

The following look like either the pipeline's environment isn't able to stand up the SMTP sink server, and/or has a conflict on port 1025. Not sure how to proceed on that score, but if it's not a quick/obvious fix for someone else I'll dig into that later.

FAILED src/pds_doi_service/core/actions test/roundup_test.py::WeeklyRoundupEmailNotificationTestCase::test_attachment_content
FAILED src/pds_doi_service/core/actions/test/roundup_test.py::WeeklyRoundupEmailNotificationTestCase::test_html_content
FAILED src/pds_doi_service/core/actions/test/roundup_test.py::WeeklyRoundupEmailNotificationTestCase::test_roundup_email_recipients_correct
FAILED src/pds_doi_service/core/actions/test/roundup_test.py::WeeklyRoundupEmailNotificationTestCase::test_roundup_email_sender_correct

[jordanpadams]

@alexdunnjpl do we want to try to spin up a fake SMTP server, e.g. https://github.com/upgundecha/start-sendria-github-action?

[jordanpadams]

@alexdunnjpl if that doesn't work, I would just disable those test? or update the github action to ignore that test?

[alexdunnjpl]

@alexdunnjpl if that doesn't work, I would just disable those test? or update the github action to ignore that test?

@jordanpadams I don't think that would work, as the test relies on capturing the stdout from the python smtp DebuggingServer, and you wouldn't want to introduce sendria as a dependency just for that test because it'd be a minor pain for local unit-testing.

It would be possible to write a test that checked DebuggingServer output and sendria, and failed iff both fail, but that seems dirty and like a lot of effort just to support a few tests specific to one script which is unlikely to change often and which is/should-be subject to local unit testing anyway.

My recommendation would be to set the tests as ignored for the time being and have me open a ticket to look into why the DebuggingServer isn't working, at a later date.

[alexdunnjpl]

@jordanpadams @nutjob4life can I get a sanity-check on this?

yet

     _____________ WeeklyRoundupEmailNotificationTestCase.test_is_in_ci _____________

self = <pds_doi_service.core.actions.test.roundup_test.WeeklyRoundupEmailNotificationTestCase testMethod=test_is_in_ci>

    def test_is_in_ci(self):
>       self.assertEqual(os.environ.get('CI'), 'edunntag')
E       AssertionError: None != 'edunntag'

That env var should be set and available in the python environment, right?

[nutjob4life]

I can confirm that forked processes from the Roundup Action do properly inherit the environment. Roundup Action doesn't do anything special with the environment variables.

I'm guessing that maybe tox is replacing the environment to ensure runs are idempotent and repeatable 🤷‍♀️ (I tested with a project that uses the older python setup.py test approach and a log message in setup.py itself properly printed the value of CI).

But this test would fail even if tox passed through the value for CI:

self.assertEqual(os.environ.get('CI'), 'edunntag')

Because true ≠ edunntag.

Maybe just drop this test? Seems like a lot of fretting and consternation over something so small.

EDIT: Yep, it's tox. Apparently there's a passenv option that tells what env vars to pass through from the running environment.

[alexdunnjpl]

But this test would fail even if tox passed through the value for CI:

self.assertEqual(os.environ.get('CI'), 'edunntag')

Because true ≠ edunntag.

Maybe just drop this test? Seems like a lot of fretting and consternation over something so small.

Sorry, I was unclear - the edunntag thing was only there to force the test log to dump the value (in this case, None)

EDIT: Yep, it's tox. Apparently there's a passenv option that tells what env vars to pass through from the running environment.

Ah, thanks - will set it up to pass through CI.

[alexdunnjpl]

Ready to merge as soon as Jordan has chimed in on the week timezone question.

[nutjob4life]

Sorry, I was unclear - the edunntag thing was only there to force the test log to dump the value (in this case, None)

Ah OK. edunntag … what an odd word! What's its origin?

[jordanpadams]

@alexdunnjpl so sorry to do this, but after talking to some other stakeholders, we came to the realization the modification date will not be accurate for "Available" date, since they may have modified it in September, but it was not released until December. Can we comment out that code for now until we have a better idea of how we will get this date from the metadata?

[alexdunnjpl]

Sorry, I was unclear - the edunntag thing was only there to force the test log to dump the value (in this case, None)

Ah OK. edunntag … what an odd word! What's its origin?

User edunn, and it's a unique tag for me to find in the logs!

@alexdunnjpl so sorry to do this, but after talking to some other stakeholders, we came to the realization the modification date will not be accurate for "Available" date, since they may have modified it in September, but it was not released until December. Can we comment out that code for now until we have a better idea of how we will get this date from the metadata?

Different PR/Issue, but sure can!