Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[HttpKernel] tweaked redirection profiling in RequestDataCollector#18618

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 3 commits intosymfony:masterfromHeahDude:fix/profile-redirect
Apr 28, 2016

Conversation

@HeahDude
Copy link
Contributor

@HeahDudeHeahDude commentedApr 22, 2016
edited
Loading

QA
Branch?master
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets~
LicenseMIT
Doc PR~

@HeahDude
Copy link
ContributorAuthor

HeahDude commentedApr 22, 2016
edited
Loading

@javiereguiluz it happens because of this linehttps://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/EventListener/ProfilerListener.php#L81.

But the scope ofRequestDataCollector::collect() does not allow to know it the request is the sub request.
So I had to register theRequestDataCollector onKernelEvents::RESPONSE to get it from the event.

I also refactored a bit since$session was already defined inRequestDataCollector::collect().

Please tell me if the fix works for you too, and thanks again for reporting it :)

@HeahDudeHeahDude changed the titleFix/profile redirect[HttpKernel] tweaked redirection profiling in RequestDataCollectorApr 22, 2016
@HeahDudeHeahDudeforce-pushed thefix/profile-redirect branch 3 times, most recently from4127b5c toa47d2e8CompareApril 22, 2016 19:00
fixes redirection profile introduced in0a1b284.Prevents collecting redirect data on sub request profiling.
@HeahDude
Copy link
ContributorAuthor

HeahDude commentedApr 23, 2016
edited
Loading

I reverted the redirect controller thing as I got the wrong result while testing it withsymfony-demo.
Also I've pusheddf19c14 to store the redirected flag in the request attributes instead of the session and used the returned value ofSession::remove. If you agree with this commit I'll squash it.

Thanks.

@fabpot
Copy link
Member

ping@javiereguiluz

@fabpot
Copy link
Member

Thank you@HeahDude.

@fabpotfabpot merged commitdf19c14 intosymfony:masterApr 28, 2016
fabpot added a commit that referenced this pull requestApr 28, 2016
…ollector (HeahDude)This PR was merged into the 3.1-dev branch.Discussion----------[HttpKernel] tweaked redirection profiling in RequestDataCollector| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | ~| License       | MIT| Doc PR        | ~-c8ba3b2 removes duplicated code forgotten in#17589-a47d2e8 fixes the collecting of redirect data in first sub request instead of redirected master request.Commits-------df19c14 use a request attribute flag for redirection profileb26cb6d [HttpKernel] added RequestDataCollector::onKernelResponse()c8ba3b2 [HttpKernel] remove legacy duplicated code
'status_text' => Response::$statusTexts[(int)$statusCode],
));
}
if (isset($session)) {
Copy link
Member

Choose a reason for hiding this comment

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

Usingisset on a variable should be avoided. I've fixed it inbecdbd9

HeahDude reacted with thumbs up emoji
@HeahDudeHeahDude deleted the fix/profile-redirect branchApril 28, 2016 10:48
@fabpotfabpot mentioned this pull requestMay 13, 2016
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HeahDude@fabpot@javiereguiluz@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp