-
Notifications
You must be signed in to change notification settings - Fork 3
VED-790-Redis-Schema-Storage #867
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
Conversation
|
This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket: VED-790 |
# Conflicts: # grafana/non-prod/docker/build_push_to_ecr.sh
| logger.info("No specific transformation defined for file type: %s", file_type) | ||
| return data # Default case, return data as is if no transformation is defined | ||
|
|
||
| return transform_generic(data, file_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: previously when no transformation was defined, transform_map() returned data; now it returns {}. Is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously it threw an exception. Unrecognized formats were not stored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another question: why would we want to store unrecognised formats e.g. an arbitrary JSON file that the project does not need to retrieve from Redis?
dlzhry2nhs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Several points require addressing. Some minor, but a major issue concerning how file naming and key naming so that we will not have to rework this when it comes to components retrieving the validation schema.
lambdas/redis_sync/src/constants.py
Outdated
| class RedisCacheKey: | ||
| PERMISSIONS_CONFIG_FILE_KEY = "permissions_config.json" | ||
| DISEASE_MAPPING_FILE_KEY = "disease_mapping.json" | ||
| VALIDATION_SCHEMA_FILE_KEY = "schema.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filename should be ValidationRules.JSON
dlzhry2nhs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with the changes now - will approve once new file name is clarified.
|



Summary
Permit upload to redis cache of json schema.
,
New version feathres:
Reviews Required
Review Checklist
ℹ️ This section is to be filled in by the reviewer.