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
handle s3 timeout on settings page #4036
Conversation
lib/plausible_web/live/csv_export.ex
Outdated
assign(socket, status: "in_progress") | ||
%{storage: storage, site: site, site_id: site_id} = socket.assigns | ||
|
||
assign_async(socket, [:status, :export], fn -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with a quick-fix using assign_async
. Alternative approach is reducing the number of attempts in Exports.get_s3_export/1
to 1 and adding one more :status
for export_fetch_failure
. That would remove the loading state which is somewhat unnecessary in "happy path".
Any preference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine with assign_async
👍 Perhaps we could extract get_export
assingment and Exports.get_last_export_job/1
outside of it? Not a big deal though.
lib/plausible_web/live/csv_export.ex
Outdated
assign(socket, status: "in_progress") | ||
%{storage: storage, site: site, site_id: site_id} = socket.assigns | ||
|
||
assign_async(socket, [:status, :export], fn -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine with assign_async
👍 Perhaps we could extract get_export
assingment and Exports.get_last_export_job/1
outside of it? Not a big deal though.
prev_env = Application.get_env(:ex_aws, :s3) | ||
new_env = Keyword.update!(prev_env, :port, fn prev_port -> prev_port + 1 end) | ||
Application.put_env(:ex_aws, :s3, new_env) | ||
on_exit(fn -> Application.put_env(:ex_aws, :s3, prev_env) end) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
patch_env
only supports :plausible
app
end | ||
|
||
@tag :capture_log | ||
test "displays error message", %{conn: conn, site: site} do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll need to skip this test in CE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes
This PR reduces export-fetch request attempts to one and handles the resulting error (if there is one). This allows import feature to continue working when S3 (used in exports) is not available.
Tests
Changelog
Documentation
Dark mode