Skip to content
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.

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

Do not write test data to the history table #775

Merged
merged 5 commits into from
Jan 10, 2017

Conversation

minglis
Copy link
Contributor

@minglis minglis commented Dec 19, 2016

Ensure updates on a research mode service or test key don't touch the…
… history table

  • note this is an unexpectedly big change.
  • When we create a service we pass the service id to the persist method. This means that we don't have the service available to check if in research mode.
  • All calling methods (expecting the one where we use the notify service) have the service available. So rather than reload it I changed the method signature to pass the service, not the ID to persist.
  • Touches a few places.

Note this means that the update or create methods will fall over on a null service. But this seems correct.

Goes back to the story which we need to play to make the service available as the API user so that the need to load and pass around services is minimised.

Martyn Inglis added 4 commits December 19, 2016 13:57
1) research mode service
2) test mode key

Stop test data getting into history and subsequently into stats and so on.
… history table

- note this is an unexpectedly big change.
- When we create a service we pass the service id to the persist method. This means that we don't have the service available to check if in research mode.
- All calling methods (expecting the one where we use the notify service) have the service available. So rather than reload it I changed the method signature to pass the service, not the ID to persist.
- Touches a few places.

Note this means that the update or create methods will fall over on a null service. But this seems correct.

Goes back to the story which we need to play to make the service available as the API user so that the need to load and pass around services is minimised.
Copy link
Contributor

@leohemsted leohemsted left a comment

Choose a reason for hiding this comment

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

👍

@servingUpAces
Copy link
Contributor

I left a comment in the story https://www.pivotaltracker.com/story/show/135208989 which brings up a question of about research mode and how things can look different if you switch in and out.
I am okay with this PR being merged before addressing that question but we should take another look at it.

@minglis minglis merged commit 0c6193e into master Jan 10, 2017
@minglis minglis deleted the do-not-write-test-data-to-the-history-table branch January 10, 2017 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants