[GitHub] trafficserver pull request: TS-3909 Further trampoline crash fixes...

2016-04-15 Thread jpeach
Github user jpeach commented on the pull request: https://github.com/apache/trafficserver/pull/575#issuecomment-210700064 Is this testable? What do you think about regression tests for this? --- If your project is set up for it, you can reply to this email and have your reply appear o

[GitHub] trafficserver pull request: TS-4323 Change default DATA frame size...

2016-04-15 Thread zwoop
Github user zwoop commented on the pull request: https://github.com/apache/trafficserver/pull/572#issuecomment-210678289 Committed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this featur

[GitHub] trafficserver pull request: TS-4323 Change default DATA frame size...

2016-04-15 Thread zwoop
Github user zwoop closed the pull request at: https://github.com/apache/trafficserver/pull/572 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature i

[GitHub] trafficserver pull request: TS-3909 Further trampoline crash fixes...

2016-04-15 Thread shinrich
GitHub user shinrich opened a pull request: https://github.com/apache/trafficserver/pull/575 TS-3909 Further trampoline crash fixes. We have been running with this fix in production since November or December 2015. You can merge this pull request into a Git repository by running:

[GitHub] trafficserver pull request: TS-4183: cachekey: URI/prefix/path cap...

2016-04-15 Thread zwoop
Github user zwoop closed the pull request at: https://github.com/apache/trafficserver/pull/552 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature i

[GitHub] trafficserver pull request: TS-4183: cachekey: URI/prefix/path cap...

2016-04-15 Thread zwoop
Github user zwoop commented on the pull request: https://github.com/apache/trafficserver/pull/552#issuecomment-210630333 Committed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this featur

[GitHub] trafficserver pull request: TS-4312 Add config to strictly parse U...

2016-04-15 Thread bgaff
Github user bgaff commented on the pull request: https://github.com/apache/trafficserver/pull/574#issuecomment-210616641 :+1: This looks good to me, thanks @shenzhang920 ! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. I

[GitHub] trafficserver pull request: TS-4312 Add config to strictly parse U...

2016-04-15 Thread shenzhang920
Github user shenzhang920 commented on the pull request: https://github.com/apache/trafficserver/pull/574#issuecomment-210592225 @jpeach @bgaff I squash my previous 4 commit into 1 commit and re-submit the pull request. I also modify the commit message to be concise. --- If your proj

[GitHub] trafficserver pull request: TS-4312 Add config to strictly parse U...

2016-04-15 Thread shenzhang920
GitHub user shenzhang920 opened a pull request: https://github.com/apache/trafficserver/pull/574 TS-4312 Add config to strictly parse URL according to RFC 3986 Add "proxy.config.http.strict_uri_parsing" in records.config to enable Traffic Server to return a 400 Bad Request if client

Re: Question on CacheURL plugin

2016-04-15 Thread Gancho Tenev
+CC: dev@trafficserver.apache.org > On Apr 15, 2016, at 11:30 AM, Gancho Tenev wrote: > > Hi Muhammad, > > One way to check the value of the cache key would be to add > etc/trafficserver/plugin.config > > xdebug.so > > and them adding -H 'X-Debug: X-Cache-Key’ to the curl command-line para

[GitHub] trafficserver pull request: TS-4312: Adding config to parse urls a...

2016-04-15 Thread bgaff
Github user bgaff commented on the pull request: https://github.com/apache/trafficserver/pull/541#issuecomment-210576382 Hey @shenzhang920 why did you close the pull request? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] trafficserver pull request: TS-4312: Adding config to parse urls a...

2016-04-15 Thread shenzhang920
Github user shenzhang920 closed the pull request at: https://github.com/apache/trafficserver/pull/541 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the fe

Re: merging github pull requests

2016-04-15 Thread Phil Sorber
Also pretty sure you have to set your apache email as your primary/public if you want it to show in merges. On Fri, Apr 15, 2016 at 12:06 PM Phil Sorber wrote: > Just found this: > > Keep my email address private > > We will use *psudae...@users.noreply.github.com > * when performing web-based G

Re: merging github pull requests

2016-04-15 Thread Phil Sorber
Just found this: Keep my email address private We will use *psudae...@users.noreply.github.com * when performing web-based Git operations and sending email on your behalf. If you want command line Git operations to use your private email you must set your email in Git

Re: merging github pull requests

2016-04-15 Thread Leif Hedstrom
> On Apr 15, 2016, at 11:22 AM, James Peach wrote: > > >> On Apr 15, 2016, at 10:09 AM, Leif Hedstrom wrote: >> >> >>> On Apr 15, 2016, at 9:34 AM, Brian Geffon wrote: >>> >>> I'm not thrilled with it either. I agree that the email addresses are >>> definitely not desireable, also I don't

Re: merging github pull requests

2016-04-15 Thread Phil Sorber
This is the stuff I wanted to talk about at the summit. I think we should hold off on merging stuff until we can talk about it next month. Also, fwiw, the email addresses seem to work sometimes and not others. So there must be a misconfiguration somewhere triggering this. On Fri, Apr 15, 2016 at

Re: [trafficserver] 01/01: Merge pull request #567 from sudheerv/ts-3857

2016-04-15 Thread James Peach
I can't find an email with the actual diff? Is there one, or does that only happen on squashed merges? > On Apr 14, 2016, at 3:06 PM, sudhe...@apache.org wrote: > > This is an automated email from the ASF dual-hosted git repository. > > sudheerv pushed a commit to branch master > in repository

Re: merging github pull requests

2016-04-15 Thread Bret Palsson
Has anyone reached out to github to see if they can resolve the email address issue? Seems like with this type of integration, they should have considered this behavior. -Bret On Fri, Apr 15, 2016 at 10:09 AM, Leif Hedstrom wrote: > > > On Apr 15, 2016, at 9:34 AM, Brian Geffon wrote: > > > >

Re: merging github pull requests

2016-04-15 Thread James Peach
> On Apr 15, 2016, at 10:09 AM, Leif Hedstrom wrote: > > >> On Apr 15, 2016, at 9:34 AM, Brian Geffon wrote: >> >> I'm not thrilled with it either. I agree that the email addresses are >> definitely not desireable, also I don't like how it also produces a merge >> commit. > > > I can deal w

Re: merging github pull requests

2016-04-15 Thread Leif Hedstrom
> On Apr 15, 2016, at 9:34 AM, Brian Geffon wrote: > > I'm not thrilled with it either. I agree that the email addresses are > definitely not desireable, also I don't like how it also produces a merge > commit. I can deal with the merge commit, but the email addressing issues are not cool. W

[GitHub] trafficserver pull request: Dummy commit to test github email.

2016-04-15 Thread sudheerv
Github user sudheerv closed the pull request at: https://github.com/apache/trafficserver/pull/573 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the featur

[GitHub] trafficserver pull request: Dummy commit to test github email.

2016-04-15 Thread sudheerv
GitHub user sudheerv opened a pull request: https://github.com/apache/trafficserver/pull/573 Dummy commit to test github email. Dummy commit to test email address for github You can merge this pull request into a Git repository by running: $ git pull https://github.com/sudheerv

[GitHub] trafficserver pull request: TS-4072 Diagnostic log rolling races

2016-04-15 Thread danobi
Github user danobi commented on the pull request: https://github.com/apache/trafficserver/pull/568#issuecomment-210516157 @jpeach Thanks for the comments. I've addressed the relevant issues you brought up in the force pushed commits. I've also gone ahead and made a separate ti

[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...

2016-04-15 Thread jpeach
Github user jpeach commented on the pull request: https://github.com/apache/trafficserver/pull/503#issuecomment-210513483 I'm -1 on this unless we know it actually fixes the problem. Looking at the [TS-4240](https://issues.apache.org/jira/browse/TS-4240), my guess is that ``data[i]``

Re: merging github pull requests

2016-04-15 Thread Brian Geffon
I'm not thrilled with it either. I agree that the email addresses are definitely not desireable, also I don't like how it also produces a merge commit. On Friday, April 15, 2016, James Peach wrote: > Hi all, > > I just merged PR #563 using the "merge and squash" button on github. I'm > pretty un

merging github pull requests

2016-04-15 Thread James Peach
Hi all, I just merged PR #563 using the "merge and squash" button on github. I'm pretty unhappy with the results, and I wonder whether we should stop trying to use github to merge. The things I don't like are - github ate the Daniel's email address so the author is now "Daniel Xu " - The com

[GitHub] trafficserver pull request: TS-4250 fix

2016-04-15 Thread jpeach
Github user jpeach closed the pull request at: https://github.com/apache/trafficserver/pull/563 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature

[GitHub] trafficserver pull request: TS-4072 Diagnostic log rolling races

2016-04-15 Thread jpeach
Github user jpeach commented on the pull request: https://github.com/apache/trafficserver/pull/568#issuecomment-210501721 I don't see any reason to block writing tests on some proposed framework. All you really need to write a test is a hook to run it (which we have) and a way to chec

[GitHub] trafficserver pull request: TS-4072 Diagnostic log rolling races

2016-04-15 Thread SolidWallOfCode
Github user SolidWallOfCode commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/568#discussion_r59890598 --- Diff: lib/ts/Diags.cc --- @@ -716,14 +722,12 @@ Diags::should_roll_outputlog() bool ret_val = false; bool need_consider_stderr

[GitHub] trafficserver pull request: TS-4072 Diagnostic log rolling races

2016-04-15 Thread jpeach
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/568#discussion_r59889953 --- Diff: lib/ts/Diags.cc --- @@ -816,29 +820,39 @@ Diags::set_stdout_output(const char *_bind_stdout) if (strcmp(_bind_stdout, "") == 0)

[GitHub] trafficserver pull request: TS-4072 Diagnostic log rolling races

2016-04-15 Thread jpeach
Github user jpeach commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/568#discussion_r59889285 --- Diff: lib/ts/Diags.cc --- @@ -716,14 +722,12 @@ Diags::should_roll_outputlog() bool ret_val = false; bool need_consider_stderr = true;

[GitHub] trafficserver pull request: TS-4072 Diagnostic log rolling races

2016-04-15 Thread danobi
Github user danobi commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/568#discussion_r59888075 --- Diff: lib/ts/Diags.cc --- @@ -816,29 +820,39 @@ Diags::set_stdout_output(const char *_bind_stdout) if (strcmp(_bind_stdout, "") == 0)

[GitHub] trafficserver pull request: TS-4072 Diagnostic log rolling races

2016-04-15 Thread SolidWallOfCode
Github user SolidWallOfCode commented on the pull request: https://github.com/apache/trafficserver/pull/568#issuecomment-210488274 While I think unit tests are in general a good idea, I'm not sure how that could be done in this case and not in the time frame for committing this to 6.2

[GitHub] trafficserver pull request: TS-4240: fix coredump issues with esi ...

2016-04-15 Thread shukitchan
Github user shukitchan commented on the pull request: https://github.com/apache/trafficserver/pull/503#issuecomment-210377071 @jpeach i thought that it was safe, too. Till i see the core dump reported in the jira. i am just being extra safe in this PR. What do you think? --- If your

[GitHub] trafficserver pull request: TS-4266: Added GC collect in add modul...

2016-04-15 Thread shukitchan
Github user shukitchan commented on the pull request: https://github.com/apache/trafficserver/pull/543#issuecomment-210329030 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this featur

[GitHub] trafficserver pull request: TS-4266: Added GC collect in add modul...

2016-04-15 Thread shukitchan
Github user shukitchan closed the pull request at: https://github.com/apache/trafficserver/pull/543 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feat