Skip to content

VED-000 Quick test - will close soon#1286

Closed
dlzhry2nhs wants to merge 3 commits intomasterfrom
feature/VED-000-dy-final-pr-test
Closed

VED-000 Quick test - will close soon#1286
dlzhry2nhs wants to merge 3 commits intomasterfrom
feature/VED-000-dy-final-pr-test

Conversation

@dlzhry2nhs
Copy link
Copy Markdown
Contributor

Summary

  • Routine Change

Just a quick test. Will close the PR and handover the branch as a future reference if useful.

Reviews Required

  • Dev

Review Checklist

ℹ️ This section is to be filled in by the reviewer.

  • I have reviewed the changes in this PR and they fill all of the acceptance criteria of the ticket.
  • If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
  • If there were changes that are outside of the regular release processes e.g. account infrastructure to setup, manual setup for external API integrations, secrets to set, then I have checked that the developer has flagged this to the Tech Lead as release steps.
  • I have checked that no Personal Identifiable Data (PID) is logged as part of the changes.

@github-actions
Copy link
Copy Markdown
Contributor

This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket:

VED-000

return []
for vacc_type in vaccine_types:
# Should make these DB keys constants
key_condition = patient_key_condition & Key("PatientSK").begins_with(vacc_type)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Really interestingly, Key condition (which would be the most efficient way to run the query using the KeyCondition which uses the Partition and sort key). However, when testing (see first commit) I found that DynamoDB does not support PK equals this AND (vacc type key begins with A OR B OR C). Or conditions can only be used in the Filter Expression.

It is not explicitly called out in the docs, but sort of implied when they say that a sort key value and comparison can be provided (but not multiple): https://docs.aws.amazon.com/boto3/latest/reference/services/dynamodb/table/query.html

My approach here might ultimately be worse, because we will have to do a query per vacc type, although the query itself would be so much more efficient than what we currently are doing whereby we are retrieving all vaccinations.

Another option, besides leaving this alone and filtering client side, would be to add all the vacc type OR conditions to the FilterExpression. In truth, it's not that much more efficient as Dynamo will still retrieve all items before applying the filter, but at least it will be done server side. Will leave it up to you guys.

@sonarqubecloud
Copy link
Copy Markdown

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.

1 participant