Always run the computation of the user profile stats #1005
Merged
+10
−9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In e-mission#993 we expanded `_get_and_store_range` to store entries other than the pipeline range, notably the `last_call_ts`. However, adding it to the end of the pipeline causes it to be skipped most of the time. We have an early return from `run_intake_pipeline_for_user` if there is no new data, so the end of the pipeline (the code in e-mission#993) is effectively in the "else" That's fine as long as we were only caching pipeline state, because it wouldn't change if the pipeline was skipped But now that we are caching the last_call_ts the current location will not work. Concretely, if the app is contacting the server but is not uploading any data, last_call_ts will never be changed. Since this is a fix that I am implementing on a holiday weekend, I have not added unit tests to check this. @TeachMeTW please enhance the unit tests to verify that the stats are generated in both cases: (i) when there is new data and (ii) when there is no new data. I have also changed the two tests that were running `epi.run_intake_pipeline_for_user` and checking the profile for results, since that is not run in that function any more. I am a bit surprised that `TestUserStat` was calling `epi.run_intake_pipeline_for_user` although `etc.runIntakePipeline` was enhanced to run the new step. @TeachMeTW Why was the enhancement needed? We need to figure out which method to call and be consistent about it as part of adding new tests (@JGreenlee, @TeachMeTW)
shankari
force-pushed
the
run_user_stats_profile_always
branch
from
aac991a to
9d1f414
Compare
|
Tested on staging, stats should be computed now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #993 we expanded
_get_and_store_rangeto store entries other than the pipeline range, notably thelast_call_ts.However, adding it to the end of the pipeline causes it to be skipped most of the time.
We have an early return from
run_intake_pipeline_for_userif there is no new data, so the end of the pipeline (the code in #993) is effectively in the "else"That's fine as long as we were only caching pipeline state, because it wouldn't change if the pipeline was skipped
But now that we are caching the last_call_ts the current location will not work. Concretely, if the app is contacting the server but is not uploading any data, last_call_ts will never be changed.
Since this is a fix that I am implementing on a holiday weekend, I have not added unit tests to check this. @TeachMeTW please enhance the unit tests to verify that the stats are generated in both cases: (i) when there is new data and (ii) when there is no new data.