Skip to content
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

Fixes error when transcript file is missing (cont) #2412

Merged

Conversation

afred
Copy link
Contributor

@afred afred commented Apr 7, 2022

Loads the Rails environment in download_clean_ingest.rb instead of stubbing the Rails module.

The use of ExternalFile within the ingest context was brittle because for some
reason thed download_clean_ingest.rb is explicitly avoiding loading the Rails
environment, and instead methods on the Rails module, such as .cache, and
simply calling Rails.cache.delete in the ExternalFile class caused the ingest
script to fail.

Not sure why the Rails environment was deliberately avoided during the ingest script, other
than a way to save on memory. But it's weird, and leads to brittle code, as this case
highlights.

Loads the Rails environment in download_clean_ingest.rb instead of stubbing the Rails module.

The use of ExternalFile within the ingest context was brittle because for some
reason thed download_clean_ingest.rb is explicitly avoiding loading the Rails
environment, and instead methods on the Rails module, such as .cache, and
simply calling Rails.cache.delete in the ExternalFile class caused the ingest
script to fail.

Not sure why the Rails environment was deliberately avoided during the ingest script, other
than a way to save on memory. But it's weird, and leads to brittle code, as this case
highlights.
@afred afred force-pushed the 2408-fix-ingest-error-when-transcript-file-is-missing branch from d987575 to 7b96961 Compare April 7, 2022 16:04
@afred afred merged commit 7f78869 into master Apr 7, 2022
@afred afred deleted the 2408-fix-ingest-error-when-transcript-file-is-missing branch April 7, 2022 16:13
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