X Tutup
Skip to content

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

Merged
phil-davis merged 1 commit intomasterfrom
bugfix/connection-status-detection
Jun 12, 2020
Merged

Try to detect the connection status after flush() - this might lead to earlier detection of a broken connection#37291
phil-davis merged 1 commit intomasterfrom
bugfix/connection-status-detection

Conversation

@DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Apr 22, 2020 •
edited
Loading

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.

Sign in to view
@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 •
edited
Loading

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 commented May 18, 2020

@DeepDiver1975 please provide description

done

@phil-davis
Copy link
Contributor

phil-davis commented May 28, 2020

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

phil-davis commented Jun 12, 2020

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
@davitol davitol mentioned this pull request Jun 25, 2020
Closed
65 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jvillafanez jvillafanez jvillafanez approved these changes

@phil-davis phil-davis Awaiting requested review from phil-davis

+1 more reviewer

@micbar micbar micbar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@DeepDiver1975 DeepDiver1975

Labels

4 - To release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@DeepDiver1975 @micbar @phil-davis @mmattel @jvillafanez
X Tutup