Eng 432 lambda renaming #4094
Eng 432 lambda renaming #4094
Conversation
Part of ENG-432 Signed-off-by: Ramil Garipov <ramil@metriport.com>
|
Caution Review failedFailed to post review comments. WalkthroughThis change introduces a new Lambda and SQS queue for contributing EHR resource diff bundles, integrates this contribution step into the existing compute resource diff bundles workflow, and adds related configuration and environment variable support. It also adds support for Elation EHR, refines FHIR conversion handling, updates ESLint rules, and makes various schema, model, and infrastructure adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant API
participant Lambda_Compute
participant SQS_Contribute
participant Lambda_Contribute
participant EHR_System
participant S3
API->>Lambda_Compute: Trigger compute resource diff bundles
Lambda_Compute->>SQS_Contribute: Send message to contribute queue
SQS_Contribute->>Lambda_Contribute: Trigger contribute handler
Lambda_Contribute->>S3: Fetch EHR-only resources bundle
Lambda_Contribute->>EHR_System: Hydrate references (multiple requests)
Lambda_Contribute->>EHR_System: Contribute hydrated resource diff bundle
Lambda_Contribute->>S3: Store result/status
sequenceDiagram
participant User
participant API
participant Lambda_FHIR_Converter
participant S3
User->>API: Request FHIR conversion (CCDA input)
API->>Lambda_FHIR_Converter: Invoke Lambda with CCDA payload
Lambda_FHIR_Converter->>S3: Read templates and/or payloads
Lambda_FHIR_Converter-->>API: Return FHIR bundle
API-->>User: Return FHIR bundle
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error code ERR_SSL_WRONG_VERSION_NUMBER ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/core/src/command/patient-import/api/update-job-status.ts (1)
7-7: Same aliasing concern as noted in create-patient-mapping.ts – favour a path alias (e.g.@core/shared/api) rather than../../shared/api/shared.packages/core/src/command/patient-import/api/create-patient.ts (1)
4-4: Same aliasing concern as noted earlier – deep relative paths are brittle; suggest switching to an alias import.
🧹 Nitpick comments (1)
packages/core/src/command/patient-import/api/create-patient-mapping.ts (1)
4-4: Prefer path alias over deep relative importJumping three directories (
../../) is brittle and fragile to future refactors.
Consider exposingwithDefaultApiErrorHandlingthrough a barrel export (e.g.@core/shared/api) and reference it via the ts-configpathsmapping:-import { withDefaultApiErrorHandling } from "../../shared/api/shared"; +import { withDefaultApiErrorHandling } from "@core/shared/api";This keeps imports stable, short, and aligned with the onion-layer guideline of minimizing coupling to file structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/api/src/routes/internal/medical/patient-job.ts(1 hunks)packages/core/src/command/patient-import/api/create-patient-mapping.ts(1 hunks)packages/core/src/command/patient-import/api/create-patient.ts(1 hunks)packages/core/src/command/patient-import/api/update-job-status.ts(1 hunks)packages/core/src/command/patient-import/patient-import-shared.ts(1 hunks)packages/core/src/command/patient-import/steps/query/patient-import-query-local.ts(3 hunks)packages/core/src/command/shared/api/start-document-query.ts(1 hunks)packages/core/src/command/shared/api/start-patient-query.ts(2 hunks)packages/infra/config/env-config.ts(1 hunks)packages/infra/config/example.ts(1 hunks)packages/infra/lib/api-stack.ts(2 hunks)packages/infra/lib/jobs/jobs-scheduler.ts(1 hunks)packages/infra/lib/jobs/jobs-stack.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/infra/lib/jobs/jobs-stack.ts
🧰 Additional context used
📓 Path-based instructions (2)
`**/*`: Use eslint to enforce code style Use prettier to format code Max column ...
**/*: Use eslint to enforce code style
Use prettier to format code
Max column length is 100 chars
Multi-line comments use /** */
Top-level comments go after the import (save pre-import to basic file header, like license)
Move literals to constants declared after imports when possible
File names: kebab-case
📄 Source: CodeRabbit Inference Engine (.cursorrules)
List of files the instruction was applied to:
packages/core/src/command/patient-import/api/create-patient.tspackages/core/src/command/patient-import/api/create-patient-mapping.tspackages/core/src/command/patient-import/api/update-job-status.tspackages/core/src/command/patient-import/patient-import-shared.tspackages/infra/config/example.tspackages/api/src/routes/internal/medical/patient-job.tspackages/infra/lib/api-stack.tspackages/infra/config/env-config.tspackages/core/src/command/patient-import/steps/query/patient-import-query-local.tspackages/core/src/command/shared/api/start-document-query.tspackages/infra/lib/jobs/jobs-scheduler.tspackages/core/src/command/shared/api/start-patient-query.ts
`**/*.ts`: - Use the Onion Pattern to organize a package's code in layers - Try ...
**/*.ts: - Use the Onion Pattern to organize a package's code in layers
- Try to use immutable code and avoid sharing state across different functions, objects, and systems
- Try to build code that's idempotent whenever possible
- Prefer functional programming style functions: small, deterministic, 1 input, 1 output
- Minimize coupling / dependencies
- Avoid modifying objects received as parameter
- Only add comments to code to explain why something was done, not how it works
- Naming
- classes, enums:
PascalCase- constants, variables, functions:
camelCase- file names:
8000kebab-case- table and column names:
snake_case- Use meaningful names, so whoever is reading the code understands what it means
- Don’t use negative names, like
notEnabled, preferisDisabled- For numeric values, if the type doesn’t convey the unit, add the unit to the name
- Typescript
- Use types
- Prefer
constinstead oflet- Avoid
anyand casting fromanyto other types- Type predicates: only applicable to narrow down the type, not to force a complete type conversion
- Prefer deconstructing parameters for functions instead of multiple parameters that might be of
the same type- Don’t use
nullinside the app, only on code interacting with external interfaces/services,
like DB and HTTP; convert toundefinedbefore sending inwards into the code- Use
async/awaitinstead of.then()- Use the strict equality operator
===, don’t use abstract equality operator==- When calling a Promise-returning function asynchronously (i.e., not awaiting), use
.catch()to
handle errors (seeprocessAsyncErrorandemptyFunctiondepending on the case)- Date and Time
- Always use
buildDayjs()to createdayjsinstances- Prefer
dayjs.duration(...)to create duration consts and keep them asduration- Prefer Nullish Coalesce (??) than the OR operator (||) to provide a default value
- Avoid creating arrow functions
- Use truthy syntax instead of
in- i.e.,if (data.link)notif ('link' in data)- Error handling
- Pass the original error as the new one’s
causeso the stack trace is persisted- Error messages should have a static message - add dynamic data to MetriportError's
additionalInfoprop- Avoid sending multiple events to Sentry for a single error
- Global constants and variables
- Move literals to constants declared after imports when possible (avoid magic numbers)
- Avoid shared, global objects
- Avoid using
console.logandconsole.errorin packages other than utils, infra and shared,
and try to useout().loginstead- Avoid multi-line logs
- don't send objects as a second parameter to
console.log()orout().log()- don't create multi-line strings when using
JSON.stringify()- Use
eslintto enforce code style- Use
prettierto format code- max column length is 100 chars
- multi-line comments use
/** */- scripts: top-level comments go after the import
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
packages/core/src/command/patient-import/api/create-patient.tspackages/core/src/command/patient-import/api/create-patient-mapping.tspackages/core/src/command/patient-import/api/update-job-status.tspackages/core/src/command/patient-import/patient-import-shared.tspackages/infra/config/example.tspackages/api/src/routes/internal/medical/patient-job.tspackages/infra/lib/api-stack.tspackages/infra/config/env-config.tspackages/core/src/command/patient-import/steps/query/patient-import-query-local.tspackages/core/src/command/shared/api/start-document-query.tspackages/infra/lib/jobs/jobs-scheduler.tspackages/core/src/command/shared/api/start-patient-query.ts
🧠 Learnings (1)
packages/api/src/routes/internal/medical/patient-job.ts (1)
Learnt from: RamilGaripov
PR: metriport/metriport#3976
File: packages/api/src/routes/medical/patient.ts:541-543
Timestamp: 2025-06-18T21:05:22.256Z
Learning: In packages/api/src/routes/medical/patient.ts, inline schema definitions like cohortIdSchema are acceptable and don't need to be moved to separate schema files when the user prefers to keep them inline.
🧬 Code Graph Analysis (5)
packages/infra/lib/api-stack.ts (1)
packages/infra/lib/jobs/jobs-scheduler.ts (1)
createJobsScheduler(27-48)
packages/core/src/command/patient-import/steps/query/patient-import-query-local.ts (1)
packages/core/src/command/patient-import/patient-import-shared.ts (1)
patientImportContext(4-4)
packages/core/src/command/shared/api/start-document-query.ts (1)
packages/core/src/command/shared/api/shared.ts (1)
withDefaultApiErrorHandling(24-53)
packages/infra/lib/jobs/jobs-scheduler.ts (4)
packages/infra/lib/shared/lambda-layers.ts (1)
LambdaLayers(16-16)packages/infra/config/example.ts (1)
config(8-235)packages/infra/config/env-config.ts (1)
EnvConfig(311-311)packages/infra/lib/shared/lambda-scheduled.ts (1)
createScheduledLambda(28-61)
packages/core/src/command/shared/api/start-patient-query.ts (1)
packages/core/src/command/shared/api/shared.ts (1)
withDefaultApiErrorHandling(24-53)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: check-pr / lint-build-test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (20)
packages/core/src/command/patient-import/patient-import-shared.ts (1)
4-4: LGTM! Well-structured constant addition.The new
patientImportContextconstant follows proper naming conventions and provides a clear, reusable context identifier for patient import operations.packages/infra/config/example.ts (1)
232-233: LGTM! Improved property naming clarity.The renaming of job scheduler properties enhances readability by making the "scheduled" aspect more prominent in the property names while maintaining consistency with the type definitions.
packages/infra/lib/api-stack.ts (2)
49-49: LGTM! Clean import addition.The import follows established patterns and integrates the new job scheduler functionality.
740-746: LGTM! Proper integration pattern.The
createJobsSchedulerfunction call follows the same pattern as other scheduled task integrations in the constructor, with appropriate parameter passing and placement alongside similar functionality.packages/api/src/routes/internal/medical/patient-job.ts (1)
248-248: Verify that making thedatafield required won't break existing API calls.Making a previously optional schema field required is a breaking change. Please ensure that all callers of this endpoint have been updated to include the
datafield in their request bodies.Run the following script to verify all API calls to this endpoint include the data field:
#!/bin/bash # Description: Search for API calls to the runtime data update endpoint to verify data field usage # Search for calls to the runtime data endpoint echo "Searching for API calls to runtime-data endpoint..." rg -A 10 -B 5 "runtime-data" --type ts --type js # Search for potential API client usage echo "Searching for updatePatientJobRuntimeData function calls..." rg -A 5 -B 5 "updatePatientJobRuntimeData" --type ts --type js # Search for fetch/axios calls to this endpoint echo "Searching for HTTP calls to patient job endpoints..." rg -A 10 "POST.*patient.*job.*runtime-data" --type ts --type jspackages/infra/config/env-config.ts (1)
285-286: LGTM! Consistent type definition updates.The property renaming maintains consistency with the example configuration and improves clarity by emphasizing the "scheduled" nature of these job-related properties.
packages/core/src/command/patient-import/steps/query/patient-import-query-local.ts (3)
7-9: LGTM: Import updates are consistent with the API refactoringThe import path updates and addition of
patientImportContextalign well with the centralized context management approach.
39-39: Good: Context parameter properly integratedThe
patientImportContextis correctly passed to both API calls, providing consistent context for error handling and logging across the patient import workflow.Also applies to: 48-48
52-56: Good: Dynamic context in error messagesThe error logging now uses the dynamic
patientImportContextinstead of hardcoded strings, improving consistency and maintainability.packages/core/src/command/shared/api/start-document-query.ts (3)
14-14: LGTM: Context parameter properly integratedThe addition of the required
contextparameter enhances error handling and logging capabilities. The function signature change is well-documented and consistent with the companionstartPatientQueryfunction.Also applies to: 22-22, 29-30
37-37: Good: Dynamic context in error handlingThe error messages and additional info now properly incorporate the
contextparameter, providing better traceability and debugging information.Also applies to: 43-43
30-30: Verify impact of return type changeThe function now returns
{ requestId: string }instead ofvoid. While this provides useful tracking information, ensure all callers can handle the new return type.#!/bin/bash # Description: Verify all callers of startDocumentQuery can handle the new return type # Expected: Callers either ignore the return value or properly handle the object structure echo "Searching for startDocumentQuery usage patterns..." rg -A 3 -B 1 'startDocumentQuery\(' --type tsAlso applies to: 47-47
packages/core/src/command/shared/api/start-patient-query.ts (3)
6-6: Good: Improved documentation clarityThe documentation now explicitly mentions "patient discovery query" which provides better clarity about the function's purpose.
Also applies to: 12-13
20-21: LGTM: Consistent context parameter integrationThe
contextparameter is properly integrated, matching the pattern established instartDocumentQuery. This ensures consistent error handling across both API functions.Also applies to: 26-27
38-38: Good: Consistent error handling and return typeThe error handling and return type changes are consistent with
startDocumentQuery, maintaining a uniform API pattern across related functions.Also applies to: 44-44, 48-48
packages/infra/lib/jobs/jobs-scheduler.ts (5)
10-16: LGTM: Well-defined props interfaceThe
JobsSchedulerPropstype clearly defines all required dependencies with appropriate CDK types and optional alarm configuration.
18-25: Good: Clean configuration mergingThe
getSettingshelper function cleanly merges props with configuration, maintaining separation of concerns and making the main function more readable.
31-47: LGTM: Proper Lambda creation with all required parametersThe function correctly passes all necessary parameters to
createScheduledLambda, including the shared layer, VPC configuration, and alarm action. The destructuring approach keeps the code clean and readable.
27-29: Ensure undefined return value is handled by callersThe function returns
undefinedwhenconfig.jobsis missing. Verify that all callers properly handle this case.#!/bin/bash # Description: Find all callers of createJobsScheduler to verify undefined handling # Expected: Callers should either check for undefined or handle it gracefully echo "Searching for createJobsScheduler usage..." rg -A 5 -B 2 'createJobsScheduler\(' --type ts
23-23: Verify HTTP usage for internal communicationThe URL construction uses
http://for internal API communication. Ensure this is appropriate for your infrastructure security requirements.What are the security best practices for internal AWS Lambda to API communication? Should HTTPS be used even for internal VPC communication?
…t/metriport into eng-432-lambda-renaming Signed-off-by: Thomas Yopes <thomasyopes@Thomass-MBP.attlocal.net>
Issues:
Dependencies
Description
Testing
imo, N/A
Release Plan
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores
Style
Tests
This release brings enhanced interoperability, improved developer tooling, and greater reliability in EHR data processing workflows.