Skip to content

LG-14030: Ensure roll plan 0032's rake task is available in all environments#10988

Merged
eileen-nava merged 5 commits intomainfrom
em/14030-restore-backfill-sponsor-id-rake-task
Jul 29, 2024
Merged

LG-14030: Ensure roll plan 0032's rake task is available in all environments#10988
eileen-nava merged 5 commits intomainfrom
em/14030-restore-backfill-sponsor-id-rake-task

Conversation

@eileen-nava
Copy link
Contributor

Context: We need to run roll plan 0032 again before enhanced IPP launches, because we caught a bug where sponsor_id was not being set for all in_person_enrollments. PR #10984 fixed the bug. As noted in this jira comment on LG-13578, there are now in_person_enrollments with a nil sponsor_id in multiple environments. As a result, we need this rake task to be available in all environments.

🎫 Ticket

LG-14030: Ensure roll plan 0032's rake task is available in all environments

🛠 Summary of changes

  1. Reintroduce the roll plan we need to run for roll plan 0032.

📜 Testing Plan

Confirmed automated tests pass.

# bundle exec rake in_person_enrollments:backfill_sponsor_id
#
task backfill_sponsor_id: :environment do |_task, _args|
with_timeout do
Copy link
Contributor

Choose a reason for hiding this comment

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

are we able to use the Reports::BaseReport.transaction_with_timeout here?

Suggested change
with_timeout do
Reports::BaseReport.transaction_with_timeout do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reports::BaseReport.transaction_with_timeout is new to me - I'm curious what it adds here?

Copy link
Contributor

Choose a reason for hiding this comment

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

not having to re-implement a nearly-identical method below in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at the config for dev, int, and staging and found that report_timeout was not set for any of those environments. When I checked in application.yml.default, the only environment that has a value set for report_timeout is production. This makes me think that in order to use the Reports::BaseReport.transaction_with_timeout method, whoever runs this task would first have to set the report_timeout config variable in dev, int, and staging. Is this correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

when the value is nil, it's an unlimited timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL!
(Do we want an unlimited timeout?)

Copy link
Contributor

Choose a reason for hiding this comment

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

sometimes we do! but we could also update the method to accept an explicit timeout if we wanted to hardcode that. I guess what I was aiming to get at was it might be nice to DRY some timeout code, but if this rake task is going away, maybe not worth the effort

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think the reason I’m balking at this proposed change is because it feels weird to have a piece of code that isn’t related to reports rely on the report_timeout variable.

Copy link
Contributor

@jennyverdeyen jennyverdeyen left a comment

Choose a reason for hiding this comment

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

Ran the tests, looks good! LGTM!

Copy link
Contributor

@shanechesnutt-ft shanechesnutt-ft left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍🏻

@eileen-nava eileen-nava merged commit 561ac6d into main Jul 29, 2024
@eileen-nava eileen-nava deleted the em/14030-restore-backfill-sponsor-id-rake-task branch July 29, 2024 13:50
mitchellhenke pushed a commit that referenced this pull request Jul 31, 2024
…onments (#10988)

* restore rake task to backfill sponsor_id column in in_person_enrollments table

* Changelog: Upcoming Features, In-person proofing, backfill sponsor id again

* update in_person_enrollments factory and get tests passing

* do clean up
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.

4 participants