Skip to content

EMR: diff uploads #269

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 18, 2020
Merged

EMR: diff uploads #269

merged 6 commits into from
Sep 18, 2020

Conversation

@vishakha1812
Copy link
Contributor

@vishakha1812 vishakha1812 commented Sep 10, 2020

No description provided.

@vishakha1812
Copy link
Contributor Author

vishakha1812 commented Sep 10, 2020

This has some issues. Everytime the pipeline is run, the ./receiving folder is generating all the files from day 1 -- which should not be the case as the cache is the designated repository which will contain all the historical files according to the diff utilities package. This raises a question: whether EMR is generating different outputs everytime it is being run?

eujing
eujing reviewed Sep 10, 2020
View reviewed changes
@vishakha1812 vishakha1812 marked this pull request as ready for review September 10, 2020 20:22
@vishakha1812 vishakha1812 requested a review from eujing September 10, 2020 20:22
@vishakha1812
Copy link
Contributor Author

vishakha1812 commented Sep 10, 2020

Update: Ready for review!

eujing
eujing suggested changes Sep 16, 2020
View reviewed changes
Copy link
Contributor

@eujing eujing left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some error in running the tests on my side, I think mostly in the test params.json.template

Also, it seems that the archive differ is running in EMRHospSensorUpdator.update_sensor(), which is being run in a test function TestEMRHospSensorUpdator.test_update_sensor() . This test function should be using mock_s3 too to prevent real interaction with the S3 server.

@vishakha1812 vishakha1812 requested a review from eujing September 17, 2020 21:37
eujing
eujing approved these changes Sep 17, 2020
View reviewed changes
Copy link
Contributor

@eujing eujing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, also all tests pass on my side!

@krivard krivard merged commit 3f64649 into main Sep 18, 2020
@krivard krivard deleted the diff_emr branch October 29, 2020 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@eujing eujing eujing approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vishakha1812 @eujing @krivard