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

Try to detect the connection status after flush() - this might lead to earlier detection of a broken connection #37291

Merged
merged 1 commit into from
Jun 12, 2020

Conversation

DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Apr 22, 2020

Description

Bugfix: Earlier detection of connection status

On public video streaming the connection is detected to reduce server load #37219
To optimize this the connection status is queried after flush()

Related Issue

#37291

How Has This Been Tested?

Live at customer ;-)
same test steps as in #37291

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@update-docs

This comment has been minimized.

@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/connection-status-detection branch from 7378676 to fb51aac Compare April 22, 2020 07:48
@DeepDiver1975 DeepDiver1975 changed the title Set ignore_user_abort(true) and try to detect the connection status a… Try to detect the connection status after flush() - this might lead to earlier detection of a broken connection Apr 22, 2020
@DeepDiver1975 DeepDiver1975 self-assigned this Apr 22, 2020
@codecov
Copy link

codecov bot commented Apr 22, 2020

Codecov Report

Merging #37291 into master will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #37291   +/-   ##
=========================================
  Coverage     64.68%   64.68%           
  Complexity    19336    19336           
=========================================
  Files          1278     1278           
  Lines         75562    75562           
  Branches       1331     1331           
=========================================
  Hits          48876    48876           
  Misses        26294    26294           
  Partials        392      392           
Flag Coverage Δ Complexity Δ
#javascript 54.14% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.84% <0.00%> (ø) 19336.00 <0.00> (ø)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/View.php 83.36% <0.00%> (ø) 391.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa0263d...22f8690. Read the comment docs.

@micbar
Copy link
Contributor

micbar commented Apr 27, 2020

@DeepDiver1975 please provide description

@DeepDiver1975 DeepDiver1975 marked this pull request as ready for review May 18, 2020 07:37
@DeepDiver1975
Copy link
Member Author

@DeepDiver1975 please provide description

done

@phil-davis
Copy link
Contributor

This PR was done in the time when we had trouble getting codecov/patch results from codecov.

@DeepDiver1975 rebase to get a fresh run of CI?
Or override and merge?

@mmattel
Copy link
Contributor

mmattel commented Jun 12, 2020

Any progress?
Merging because already approved and CI is green?

@phil-davis phil-davis force-pushed the bugfix/connection-status-detection branch from fb51aac to 22f8690 Compare June 12, 2020 08:38
@phil-davis phil-davis self-requested a review June 12, 2020 08:38
@phil-davis
Copy link
Contributor

I rebased because it will be nice to get a really current run of CI.
I requested a review from myself so that I see this again in my "review requests". Hopefully I can "just merge it" in a hour or so.

@phil-davis phil-davis merged commit 3501040 into master Jun 12, 2020
@delete-merged-branch delete-merged-branch bot deleted the bugfix/connection-status-detection branch June 12, 2020 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants