Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/810#discussion_r72265027
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1583,7 +1583,10 @@ SSLInitServerContext(const SSLConfigParams *params,
const ssl_user_config
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/821
---
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 user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/826
:+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 feature
enabled and wishes so
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/825#discussion_r72200488
--- Diff: proxy/ProxyClientTransaction.h ---
@@ -211,7 +211,11 @@ class ProxyClientTransaction : public VConnection
virtual bool
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/825#discussion_r72140971
--- Diff: proxy/ProxyClientTransaction.h ---
@@ -211,7 +211,11 @@ class ProxyClientTransaction : public VConnection
virtual bool
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/753
[approve ci]
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/825
:+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 feature
enabled and wishes so
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/825#discussion_r72022413
--- Diff: proxy/ProxyClientTransaction.h ---
@@ -211,7 +211,11 @@ class ProxyClientTransaction : public VConnection
virtual bool
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/769
[approve ci]
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/802
@alhonen Thoughts on the concerns from James?
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/701
@oknet How do you want to proceed with this? Are you making an update to
the PR to address some of the review suggestions?
---
If your project is set up for it, you can reply to this email
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/753
@shinrich Should we continue with this PR? @oknet there are now merge
conflicts, can you please rebase (and remember to re-run clang-format too :).
---
If your project is set up for it, you
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/766
@shinrich Should we land this? Also, is this a back port candidate?
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/822
[approve ci]
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/822
Cool. Couple of things:
1) Please email dev@ with a short description for the API proposal. For
some details on the process of adding new APIs, see
https://cwiki.apache.org/confluence
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/823
@oknet This failed clang-format, please fix and push update :).
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/823
Certainly seems reasonable to me. [approve ci]
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/771
[approve ci]
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/712
Some minor details here:
1) Update the commit subject message please.
2) It now has marge conflicts, so please rebase, and re-run clang-format as
well.
@ushachar Can you
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/624
Status on this PR? Is there a Jira? The branch has conflicts now, and
likely might need a clang-format run as well.
---
If your project is set up for it, you can reply to this email and have
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/568
[approve ci] @PSUdaemon @jpeach thoughts?
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/821
@mlibbey Yes. Updated, see second commit. (I will squash the commits
before committing)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/821
TS-4678 Promotes a few plugins to stable
The following are moved:
authproxy
background_fetch
esi
generator
regex_revalidate
s3_auth
xdebug
You
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/821
@mlibbey Please review.
---
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
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/819
---
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 user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/647
@reveller Any update? :)
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/771
@oknet This fails on the clang-format, can you please fix and push an
update?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/771
@bryancall Should we merge this? [approve ci].
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/776
@bgaff We should decide what to do here, land this, land your version, or
merge ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/792
---
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 user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/792
[approve ci]
---
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
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/818
---
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 user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/810
@shinrich I think you linked this to the wrong Jira number? It should be
TS-4679, right ?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/815
---
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 user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/818
Made the changes per Gancho's suggestions.
---
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
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/819#discussion_r71749709
--- Diff: lib/ts/ink_config.h.in ---
@@ -127,5 +127,6 @@
#define TS_BUILD_CANONICAL_HOST "@host@"
#define TS_BUILD_DEFAULT_LOOP
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/818#discussion_r71722683
--- Diff: lib/ts/tests.cc ---
@@ -19,7 +19,7 @@
limitations under the License.
*/
-#include "ts/TestBox.h"
+#i
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/819
TS-4311 Removes support for SPDY completely
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4311
Alternatively you
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/817
---
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 user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/588
So, now that we're on the 7.0.0 release cycle, I wonder if we should have
pqhl, cqhl, pshl, and sshl all exclude any @ headers? If not, if we really want
this, don't we also want all 4 variants
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/310
---
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 user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/568
Well, this slipped through the net. @danobi Can you please make an update
to the PR, that does not have merge conflicts?
---
If your project is set up for it, you can reply to this email
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/310
I'm going to close this for now, please open a new PR after we discuss this
more?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/815
Yeah, like that. Makes you wonder why TSPluginDirGet() was so different
though ... I've had issues before with these APIs, where transfer of ownership
was not clear. I'll take a peek in a bit
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/817
@oschaaf please review.
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/816
I don't speak Vagrant, but this seems reasonable to me.
---
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
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/818
TS-4683 Adds better error handling on config problems
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4683
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/817
TS-4667 Uses the WKS in the gzip plugin
Instead of just using strings, like "deflate". Albeit, it's functionally
the same, this is a bad pattern that we should discourage (using
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/815
Gotcha. Well, you showed me C++11 has this requirement for thread safety,
and we've said that v7.0.0 will require C++11 support, so +1 on that.
---
If your project is set up for it, you can
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/815
If I understand C++ at all, I don't think that example is thread safe? But,
moving it outside (file scope) would be. No?
---
If your project is set up for it, you can reply to this email
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/813
@jpeach @bryancall check the update please.
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/815
Interesting, so e.g.
static char *runtimedir = RecConfigReadRuntimeDir();
would be thread safe. That seems like a no-brainer to me :). +1
---
If your project is set
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/813
Ah, yeah, I'm pretty sure that entire if() can be safely eliminated (famous
last words ...).
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/813
Let me take a look, but I was going for the simplest solution that works,
with minimal risk. It's pretty finicky on the order :)
---
If your project is set up for it, you can reply
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/815
@jpeach Wdyt? Would TLS make more sense here? It would duplicate those
static strings, but avoids locks, no ?
---
If your project is set up for it, you can reply to this email and have your
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/805
---
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 user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/814
---
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 user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/805
Great, thanks! [approve ci]
---
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
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/810#discussion_r71376108
--- Diff: iocore/net/SSLUtils.cc ---
@@ -2010,8 +2013,9 @@ ssl_extract_certificate(const matcher_line
*line_info, ssl_user_config
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/814
TS-4689 Assert if Machine UUID init fails
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4689
Alternatively you
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/805
@yatsukhnenko As James suggested, can you just add a line or two to
doc/developer-guide/api/functions/TSInstallDirGet.en.rst instead of creating a
new file? This would be my preference as well
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/813
TS-4652 Fixes log field initialization, previously crippled
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4652
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/804
---
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 user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/804
Yeah, file a Jira probably. I could have sworn we've had issues before
where loop detection kicked in when it shouldn't (e.g. you legitimately proxy
to yourself). But I can find an issue
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/809
+1, although I'm trusting your call here on the internals and changes
already made :).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/805
Sounds good, can you file a new Jira for these issues?
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/805
Unless @yatsukhnenko also wants to fix the thread safety issue on this, and
all APIs? If not, file another Jira IMO.
---
If your project is set up for it, you can reply to this email and have
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/807
I wonder if we should move this out of the examples tree? It seems useful
as a debugging tool, and most people probably don't look in examples for useful
tools :).
---
If your project is set
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/808
:+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 feature
enabled and wishes so
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/804
Did a quick test of this, and it looks good:
Via: http/1.1 fedora.ogre.com[f6f5162d-5a02-4d10-ac6e-35aede508fe5]
(ApacheTrafficServer/7.0.0)
---
If your project is set up
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/804
@yatsukhnenko What do you think of a future feature (config) to disable
this Via checking completely? If I know there's not chance of a loop, why
bother checking the Via header? I guess one
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/804
I checked the core on this FreeBSD build, and it seems completely unrelated
to pretty much anything :).
```
(gdb) bt
#0 0x in ?? ()
#1 0x006ec34a
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/804
I suspect this failed for other reasons, trying again [approve ci].
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/804
[approve ci]
---
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
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/804#discussion_r71085418
--- Diff: proxy/http/HttpTransactHeaders.cc ---
@@ -750,12 +752,10 @@
HttpTransactHeaders::insert_via_header_in_request(HttpTransact::State *s, HTTPHd
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/798#discussion_r71084383
--- Diff: plugins/experimental/esi/esi.cc ---
@@ -61,6 +61,7 @@ struct OptionInfo {
};
static HandlerManager *gHandlerManager = NULL
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/798#discussion_r71084358
--- Diff: plugins/experimental/esi/lib/Variables.cc ---
@@ -357,9 +357,26 @@ Variables::_parseCookieString(const char *str, int
str_len
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/802
[approve ci]
---
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
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/804#discussion_r71084262
--- Diff: proxy/http/HttpTransactHeaders.cc ---
@@ -750,12 +752,10 @@
HttpTransactHeaders::insert_via_header_in_request(HttpTransact::State *s, HTTPHd
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/804
It's an inefficiency for starters (you go through a C-> C++ wrapper layer,
for no good reason (remember, the public APIs are all C, whereas in the core,
you can use all the C++ obje
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/806
This is a good start, but it's missing a few things (I think at least, been
a while since I looked at this). Look at how the eval() function is handling
the different cases. But at least I
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/805
[approve ci]
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/805
Also, please add some tests to proxy/InkAPITest.cc, see how the existing
tests are done for e.g. TSInstallDirGet.
---
If your project is set up for it, you can reply to this email and have
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/805
Cool. Can you add a documentation to this too? Either just add a new file,
e.g.
doc/developer-guide/api/functions/TSRuntimeDirGet.en.rst
Or, my personal preference, merge
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/804
Almost, but not quite. :) All APIs that are prefixed TS are the public
APIs, and we don't use those in the internal code. Look at the log tags in
logging that uses the internal APIs
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/803
Could we use the ats_scoped_obj here? Or some other smart ptr? I'm ok
either way, but maybe we should encourage these smart pointer patterns?
This might change memory management though
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/803
add to whitelist
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/803
This fails on clang-format, please re-run and push --force.
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/803
[approve ci]
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/644
Hmmm, well, if it doesn't work, then it's a moot point. I know people have
implemented rate limiting in plugins, it was just never open sourced. If you
don't feel strongly about it, closing
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/644
Aww, don't close it. I don't think anything absolutely wrong here, we
should land this. There are things that needs to be fixed, but they are pretty
minor (like, squashing, clang-format, typos
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/797
The patch seems fine, but I'm a little concerned why those were put in
there to begin with? Obviously some sort of sentinel at some point? As long as
we don't need those, I'm +1
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/796
:+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 feature
enabled and wishes so
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/795
:+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 feature
enabled and wishes so
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/792
Thanks @gtenev. @ftarnell can you fix the clang format error (see the linux
build above), and then we can land this.
Thanks!
---
If your project is set up for it, you can reply
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/792
[approve ci]
@gtenev Please review.
---
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
801 - 896 of 896 matches
Mail list logo
