[CPDLP-1913] Restrict participants endpoint fields #2917
Merged
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
|
Tested this on a prod env using a benchmarking script, about a 2-3x improvement in this query which is great. This should be good to ship. |
|
Can be merged once #2921 fixes the review app build 🤞 |
This endpoint is currently timing out in production for certain providers. One possible way to speed it up is by reducing the amount of columns returned to only the necessary ones.
|
Created review app at https://ecf-review-pr-2917.london.cloudapps.digital |
|
Smoke tests passed against the review app. |
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.
Context
This endpoint is currently timing out in production for certain providers. One possible way to speed it up is by reducing the amount of columns returned to only the necessary ones.
Changes proposed in this pull request
Restrict the fields being returned by the query underlying the
/api/v1/participants/ecfendpoint.Guidance to review
Before we merge this, it would be best to test it if possible to see if it has the expected effect of speeding up the endpoint when dealing with a prod-like data load.
This can be done locally, if we can get a prod-like database running with a similar amount of data, or it can be done on a prod-like env.
It's lacking a test, partly because I want to try it out first to see if it improves performance before I invest further, but also because testing for the removal of fields always feels weird to me. Checking if a field isn't found in a payload can easily be a false positive test. If someone has a better idea for a good way to test this, please shout and we can update this.