Skip to content

Conversation

@JamesDemeryNava
Copy link
Contributor

@JamesDemeryNava JamesDemeryNava commented Dec 2, 2025

JIRA Ticket:
BB2-4271

What Does This PR Do?

This ticket takes the migrate_fhir_id_to_v2 out of a migration, and puts it into a command. This ensures we won't try to update every crosswalk record in a single transaction. We also chunk the retrieval of crosswalk records, which should improve efficiency.

It also ensures we can create hundreds of thousands of crosswalk (and auth_user) records locally, so the DB we are testing against locally more closely resembles what is in prod.

@clewellyn-nava made all of the changes outside of using objects.iterator(chunk_size=1000) instead of .all()

What Should Reviewers Watch For?

  • Anything we are not considering around moving migrate_fhir_id_to_v2 out of the 0009 bluebutton migration? Any chance of locks given this change?
  • It took just over 9 minutes to update 675,000 crosswalk records using migrate_fhir_id_to_v2. Not sure how long it will take in prod.

If you're reviewing this PR, please check for these things in particular:

Validation

  • Do all unit and integration tests pass?
  • Are you able to run the following command successfully? Note that it will create crosswalk, auth_user, and application records in your local DB
  • To fully test it, roll back the 0009 bluebutton migration, run the command below, then run the migrate_fhir_id_to_v2 command, then run the 0009 migration. Order:
    • python -m manage migrate bluebutton 0008_add_v2_v3_fhir_id
    • python -m manage create_test_users_and_applications_batch --auto-generate true
    • (on the DB) UPDATE bluebutton_crosswalk SET fhir_id_v2 = null;
    • python -m manage migrate_fhir_id_to_v2
    • python -m manage migrate bluebutton 0009_migrate_fhir_id_to_v2
  • If the create_test_users_and_applications_batch command is run for 675000 (as it is by default), it is likely that the script will never exit on its own. However, I found that all DB records necessary to test migrate_fhir_id_to_v2 were created

What Security Implications Does This PR Have?

Please indicate if this PR does any of the following:

  • Adds any new software dependencies
  • Modifies any security controls
  • Adds new transmission or storage of data
  • Any other changes that could possibly affect security?
  • Yes, one or more of the above security implications apply. This PR must not be merged without the ISSO or team
    security engineer's approval.

Any Migrations?

There's a modified migration, but we just remove a call to a different command.

  • Yes, there are migrations
    • The migrations should be run PRIOR to the code being deployed
    • The migrations should be run AFTER the code is deployed
    • There is a more complicated migration plan (downtime,
      etc)
  • No migrations

jimmyfagan
jimmyfagan previously approved these changes Dec 3, 2025
Copy link
Contributor

@jimmyfagan jimmyfagan left a comment

Choose a reason for hiding this comment

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

Approved, we'll double check with Connor before things go to sandbox/prod, but everything looks good from my perspective.

Copy link
Contributor

@jadudm jadudm left a comment

Choose a reason for hiding this comment

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

Re-ran the tests/steps on the PR. All passed/worked. LGTM.

@JamesDemeryNava JamesDemeryNava merged commit 1fcc60e into master Dec 3, 2025
8 checks passed
@JamesDemeryNava JamesDemeryNava deleted the clewellyn-nava/bb2-4271 branch December 3, 2025 17:58
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.

5 participants