[ETL-329] delete check

[rxu17]

Purpose: Expands check for deleted records in all other Healthkit data types in get_metadata:

• HealthKitV2Heartbeat
• HealthKitV2Electrocardiogram
• HealthKitV2Workouts

Also adds tests, and Dockerfile for testing the changes

[philerooski]

Just needs some minor changes in the tests. Also, what are the empty __init__.py files for? Can we get rid of them to reduce clutter?

[philerooski]

Have you seen f-strings? They're super slick: https://realpython.com/python-f-strings/#f-strings-a-new-and-improved-way-to-format-strings-in-python

[thomasyu888]

Nit: I definitely prefer to use f-string when possible, but personally, the double/single quote is annoying

f"{metadata['type']}_Deleted"

we can...

meta_type = metadata['type']
f"{meta_type}_Deleted"

I am also ok with .format in this specific scenario.

[rxu17]

Yup, I have seen it, but thought that in this scenario, it would look a bit cleaner to have .format.

[philerooski]

HealthKitV2Heartbeat_Samples_Deleted_20201022-20211022.json

[philerooski]

Use the actual format like HealthKitV2Samples_AppleExerciseTime_20230112-20230114.json

[philerooski]

HealthKitV2Electrocardiogram_Samples_Deleted_20201022-20211022.json

[rxu17]

Just needs some minor changes in the tests. Also, what are the empty __init__.py files for? Can we get rid of them to reduce clutter?

I had to add them because the test script is nested inside the subdirectories under tests/glue/jobs and adding init.py allows me to import them like src.glue.jobs import s3_to_json for example. Same goes for the lambda app script test as well. I can just remove the glue/jobs folders under tests and have these tests sit right under tests and we won't have have the blank __init__.py files.

[philerooski]

I want to try the tests for myself -- in the meanwhile, can you update line 50 of the tests README so that all tests are run and not just the lambda tests?

[rxu17]

I want to try the tests for myself -- in the meanwhile, can you update line 50 of the tests README so that all tests are run and not just the lambda tests?

Hmm I think currently the lambda tests are the only ones that can be run locally via a pipenv which is why I left that to be lambda specific otherwise you'll run into an error with pytest since test_s3_to_json.py has to be run in a Dockerfile.

[philerooski]

Makes sense. Can you include a sentence that says as much in the README?

[philerooski]

Do you also get this error when running tests?

ImportError while importing test module '/home/glue_user/workspace/recover/tests/test_s3_to_json_s3.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib64/python3.7/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/test_s3_to_json_s3.py:7: in <module>
    from src.glue.jobs import s3_to_json_s3
E   ImportError: cannot import name 's3_to_json_s3' from 'src.glue.jobs' (unknown location)

[rxu17]

Nope, you're getting this error inside the Docker image?

[philerooski]

Yeah. I figured it out. In the tests it should read:

from src.glue.jobs import s3_to_json

The file name is different than it is in BD.

[rxu17]

Yup it is different. Are you saying the tests don't have from src.glue.jobs import s3_to_json?

[philerooski]

I confused myself. I had an untracked file in tests that was using the incorrect import.

[thomasyu888]

🚀 Nice work! Thanks for all the reviews