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

Run fximport in background task #617

Merged
merged 2 commits into from
Nov 22, 2024
Merged

Run fximport in background task #617

merged 2 commits into from
Nov 22, 2024

Conversation

pajowu
Copy link
Member

@pajowu pajowu commented Nov 20, 2024

Previously we ran the importer inside the request handler, which sometimes generated timeout if many / large files were imported or the captcha solving took too long. This PR changes this to only start a task in the request handler which then runs fximport in the background and sends an email to the user on success / failure

@pajowu pajowu requested review from stefanw and krmax44 November 20, 2024 16:07
@pajowu pajowu marked this pull request as ready for review November 20, 2024 16:08
@pajowu pajowu force-pushed the pajowu/fximport_as_task branch from 11d8790 to 8400602 Compare November 20, 2024 16:24
@pajowu pajowu force-pushed the pajowu/fximport_as_task branch from 8400602 to fb2e428 Compare November 20, 2024 16:28
Copy link
Member

@stefanw stefanw left a comment

Choose a reason for hiding this comment

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

The capture_exception makes me think we should have the celery integration for Sentry. Good idea with the atomic section, we should have that in more places in tasks.

@pajowu
Copy link
Member Author

pajowu commented Nov 21, 2024

Actually i later found that using logging.exception or raise-ing after sending the mail should be enough. Will update the PR before merging. Sentry already logs tasks exceptions AFAIK

@pajowu pajowu force-pushed the pajowu/fximport_as_task branch from fb2e428 to 4dbbe80 Compare November 22, 2024 13:43
@pajowu pajowu merged commit c0f29d8 into main Nov 22, 2024
3 checks passed
@pajowu pajowu deleted the pajowu/fximport_as_task branch November 22, 2024 13:43
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.

2 participants