You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2015/04/02 00:50:35 UTC

trafficserver git commit: TS-3479 Remove verbose Warning() from HTTP/2 connections

Repository: trafficserver
Updated Branches:
  refs/heads/master 0b3fac4a3 -> 26f438d48


TS-3479 Remove verbose Warning() from HTTP/2 connections


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/26f438d4
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/26f438d4
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/26f438d4

Branch: refs/heads/master
Commit: 26f438d486eaa04267db953d8d5f1249a63af2c3
Parents: 0b3fac4
Author: Leif Hedstrom <zw...@apache.org>
Authored: Wed Apr 1 16:50:30 2015 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Wed Apr 1 16:50:30 2015 -0600

----------------------------------------------------------------------
 CHANGES                           | 2 ++
 proxy/http2/Http2SessionAccept.cc | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 7422d2c..4fc7b6e 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 6.0.0
 
+  *) [TS-3479] Remove verbose Warning() from HTTP/2 connections.
+
   *) [TS-3470] traffic_server --help segfaults when SPDY is enabled.
 
   *) [TS-1611] Async request suport in lua plugin.

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/proxy/http2/Http2SessionAccept.cc
----------------------------------------------------------------------
diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
index 5fe2ce7..fbb25db 100644
--- a/proxy/http2/Http2SessionAccept.cc
+++ b/proxy/http2/Http2SessionAccept.cc
@@ -40,7 +40,7 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
 {
   // XXX we need to refactor the ACL checks from HttpSessionAccept so that we can invoke them here, and also in
   // the SPDY protocol layer ...
-  Warning("skipping access control checks for HTTP/2 connection");
+  // Warning("skipping access control checks for HTTP/2 connection");
 
   netvc->attributes = this->options.transport_type;
 


Re: trafficserver git commit: TS-3479 Remove verbose Warning() from HTTP/2 connections

Posted by James Peach <jp...@apache.org>.
> On Apr 2, 2015, at 8:55 PM, Leif Hedstrom <zw...@apache.org> wrote:
> 
>> 
>> On Apr 2, 2015, at 9:51 PM, Leif Hedstrom <zw...@apache.org> wrote:
>> 
>>> 
>>> On Apr 1, 2015, at 6:08 PM, James Peach <jp...@apache.org> wrote:
>>> 
>>> 
>>>> On Apr 1, 2015, at 3:50 PM, zwoop@apache.org wrote:
>>>> 
>>> [snip]
>>>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/proxy/http2/Http2SessionAccept.cc
>>>> ----------------------------------------------------------------------
>>>> diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
>>>> index 5fe2ce7..fbb25db 100644
>>>> --- a/proxy/http2/Http2SessionAccept.cc
>>>> +++ b/proxy/http2/Http2SessionAccept.cc
>>>> @@ -40,7 +40,7 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
>>>> {
>>>> // XXX we need to refactor the ACL checks from HttpSessionAccept so that we can invoke them here, and also in
>>>> // the SPDY protocol layer ...
>>>> -  Warning("skipping access control checks for HTTP/2 connection");
>>>> +  // Warning("skipping access control checks for HTTP/2 connection");
>>> 
>>> I added this warning so that someone would wire up the access control checks for HTTP/2. I don't think we should remove the warning without doing that :)
>> 
>> 
>> I disagree, the Warning() makes the entire HTTP2 feature useless since it’ll fill up the disk :/.

yeh filling up the disk is bad

>> Feel free to make it a single warning on startup maybe ?
> 
> 
> I filed a Jira though, since there was none.

thanks .. and thanks for the other patch ;)

J

Re: trafficserver git commit: TS-3479 Remove verbose Warning() from HTTP/2 connections

Posted by James Peach <jp...@apache.org>.
> On Apr 2, 2015, at 8:55 PM, Leif Hedstrom <zw...@apache.org> wrote:
> 
>> 
>> On Apr 2, 2015, at 9:51 PM, Leif Hedstrom <zw...@apache.org> wrote:
>> 
>>> 
>>> On Apr 1, 2015, at 6:08 PM, James Peach <jp...@apache.org> wrote:
>>> 
>>> 
>>>> On Apr 1, 2015, at 3:50 PM, zwoop@apache.org wrote:
>>>> 
>>> [snip]
>>>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/proxy/http2/Http2SessionAccept.cc
>>>> ----------------------------------------------------------------------
>>>> diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
>>>> index 5fe2ce7..fbb25db 100644
>>>> --- a/proxy/http2/Http2SessionAccept.cc
>>>> +++ b/proxy/http2/Http2SessionAccept.cc
>>>> @@ -40,7 +40,7 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
>>>> {
>>>> // XXX we need to refactor the ACL checks from HttpSessionAccept so that we can invoke them here, and also in
>>>> // the SPDY protocol layer ...
>>>> -  Warning("skipping access control checks for HTTP/2 connection");
>>>> +  // Warning("skipping access control checks for HTTP/2 connection");
>>> 
>>> I added this warning so that someone would wire up the access control checks for HTTP/2. I don't think we should remove the warning without doing that :)
>> 
>> 
>> I disagree, the Warning() makes the entire HTTP2 feature useless since it’ll fill up the disk :/.

yeh filling up the disk is bad

>> Feel free to make it a single warning on startup maybe ?
> 
> 
> I filed a Jira though, since there was none.

thanks .. and thanks for the other patch ;)

J

Re: trafficserver git commit: TS-3479 Remove verbose Warning() from HTTP/2 connections

Posted by Leif Hedstrom <zw...@apache.org>.
> On Apr 2, 2015, at 9:51 PM, Leif Hedstrom <zw...@apache.org> wrote:
> 
>> 
>> On Apr 1, 2015, at 6:08 PM, James Peach <jp...@apache.org> wrote:
>> 
>> 
>>> On Apr 1, 2015, at 3:50 PM, zwoop@apache.org wrote:
>>> 
>> [snip]
>>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/proxy/http2/Http2SessionAccept.cc
>>> ----------------------------------------------------------------------
>>> diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
>>> index 5fe2ce7..fbb25db 100644
>>> --- a/proxy/http2/Http2SessionAccept.cc
>>> +++ b/proxy/http2/Http2SessionAccept.cc
>>> @@ -40,7 +40,7 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
>>> {
>>> // XXX we need to refactor the ACL checks from HttpSessionAccept so that we can invoke them here, and also in
>>> // the SPDY protocol layer ...
>>> -  Warning("skipping access control checks for HTTP/2 connection");
>>> +  // Warning("skipping access control checks for HTTP/2 connection");
>> 
>> I added this warning so that someone would wire up the access control checks for HTTP/2. I don't think we should remove the warning without doing that :)
> 
> 
> I disagree, the Warning() makes the entire HTTP2 feature useless since it’ll fill up the disk :/. Feel free to make it a single warning on startup maybe ?


I filed a Jira though, since there was none.

- Leif


Re: trafficserver git commit: TS-3479 Remove verbose Warning() from HTTP/2 connections

Posted by Leif Hedstrom <zw...@apache.org>.
> On Apr 2, 2015, at 9:51 PM, Leif Hedstrom <zw...@apache.org> wrote:
> 
>> 
>> On Apr 1, 2015, at 6:08 PM, James Peach <jp...@apache.org> wrote:
>> 
>> 
>>> On Apr 1, 2015, at 3:50 PM, zwoop@apache.org wrote:
>>> 
>> [snip]
>>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/proxy/http2/Http2SessionAccept.cc
>>> ----------------------------------------------------------------------
>>> diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
>>> index 5fe2ce7..fbb25db 100644
>>> --- a/proxy/http2/Http2SessionAccept.cc
>>> +++ b/proxy/http2/Http2SessionAccept.cc
>>> @@ -40,7 +40,7 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
>>> {
>>> // XXX we need to refactor the ACL checks from HttpSessionAccept so that we can invoke them here, and also in
>>> // the SPDY protocol layer ...
>>> -  Warning("skipping access control checks for HTTP/2 connection");
>>> +  // Warning("skipping access control checks for HTTP/2 connection");
>> 
>> I added this warning so that someone would wire up the access control checks for HTTP/2. I don't think we should remove the warning without doing that :)
> 
> 
> I disagree, the Warning() makes the entire HTTP2 feature useless since it’ll fill up the disk :/. Feel free to make it a single warning on startup maybe ?


I filed a Jira though, since there was none.

- Leif


Re: trafficserver git commit: TS-3479 Remove verbose Warning() from HTTP/2 connections

Posted by Leif Hedstrom <zw...@apache.org>.
> On Apr 1, 2015, at 6:08 PM, James Peach <jp...@apache.org> wrote:
> 
> 
>> On Apr 1, 2015, at 3:50 PM, zwoop@apache.org wrote:
>> 
> [snip]
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/proxy/http2/Http2SessionAccept.cc
>> ----------------------------------------------------------------------
>> diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
>> index 5fe2ce7..fbb25db 100644
>> --- a/proxy/http2/Http2SessionAccept.cc
>> +++ b/proxy/http2/Http2SessionAccept.cc
>> @@ -40,7 +40,7 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
>> {
>>  // XXX we need to refactor the ACL checks from HttpSessionAccept so that we can invoke them here, and also in
>>  // the SPDY protocol layer ...
>> -  Warning("skipping access control checks for HTTP/2 connection");
>> +  // Warning("skipping access control checks for HTTP/2 connection");
> 
> I added this warning so that someone would wire up the access control checks for HTTP/2. I don't think we should remove the warning without doing that :)


I disagree, the Warning() makes the entire HTTP2 feature useless since it’ll fill up the disk :/. Feel free to make it a single warning on startup maybe ?

— leif


Re: trafficserver git commit: TS-3479 Remove verbose Warning() from HTTP/2 connections

Posted by Leif Hedstrom <zw...@apache.org>.
> On Apr 1, 2015, at 6:08 PM, James Peach <jp...@apache.org> wrote:
> 
> 
>> On Apr 1, 2015, at 3:50 PM, zwoop@apache.org wrote:
>> 
> [snip]
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/proxy/http2/Http2SessionAccept.cc
>> ----------------------------------------------------------------------
>> diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
>> index 5fe2ce7..fbb25db 100644
>> --- a/proxy/http2/Http2SessionAccept.cc
>> +++ b/proxy/http2/Http2SessionAccept.cc
>> @@ -40,7 +40,7 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
>> {
>>  // XXX we need to refactor the ACL checks from HttpSessionAccept so that we can invoke them here, and also in
>>  // the SPDY protocol layer ...
>> -  Warning("skipping access control checks for HTTP/2 connection");
>> +  // Warning("skipping access control checks for HTTP/2 connection");
> 
> I added this warning so that someone would wire up the access control checks for HTTP/2. I don't think we should remove the warning without doing that :)


I disagree, the Warning() makes the entire HTTP2 feature useless since it’ll fill up the disk :/. Feel free to make it a single warning on startup maybe ?

— leif


Re: trafficserver git commit: TS-3479 Remove verbose Warning() from HTTP/2 connections

Posted by James Peach <jp...@apache.org>.
> On Apr 1, 2015, at 3:50 PM, zwoop@apache.org wrote:
> 
[snip]
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/proxy/http2/Http2SessionAccept.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
> index 5fe2ce7..fbb25db 100644
> --- a/proxy/http2/Http2SessionAccept.cc
> +++ b/proxy/http2/Http2SessionAccept.cc
> @@ -40,7 +40,7 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
> {
>   // XXX we need to refactor the ACL checks from HttpSessionAccept so that we can invoke them here, and also in
>   // the SPDY protocol layer ...
> -  Warning("skipping access control checks for HTTP/2 connection");
> +  // Warning("skipping access control checks for HTTP/2 connection");

I added this warning so that someone would wire up the access control checks for HTTP/2. I don't think we should remove the warning without doing that :)

J

Re: trafficserver git commit: TS-3479 Remove verbose Warning() from HTTP/2 connections

Posted by James Peach <jp...@apache.org>.
> On Apr 1, 2015, at 3:50 PM, zwoop@apache.org wrote:
> 
[snip]
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26f438d4/proxy/http2/Http2SessionAccept.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http2/Http2SessionAccept.cc b/proxy/http2/Http2SessionAccept.cc
> index 5fe2ce7..fbb25db 100644
> --- a/proxy/http2/Http2SessionAccept.cc
> +++ b/proxy/http2/Http2SessionAccept.cc
> @@ -40,7 +40,7 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
> {
>   // XXX we need to refactor the ACL checks from HttpSessionAccept so that we can invoke them here, and also in
>   // the SPDY protocol layer ...
> -  Warning("skipping access control checks for HTTP/2 connection");
> +  // Warning("skipping access control checks for HTTP/2 connection");

I added this warning so that someone would wire up the access control checks for HTTP/2. I don't think we should remove the warning without doing that :)

J