Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/846
I think I see this, we only set current_reader=NULL after that assert,
right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/843#discussion_r74176854
--- Diff: plugins/experimental/carp/CarpConfig.cc ---
@@ -0,0 +1,581 @@
+/** @file
+
+ Loads the CARP configuration
+
+ @section
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/847
I assume is_done is still in use elsewhere.
ð
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/850
:+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, or if the
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/850
This is likely a back port candidate for 6.2.1 as well.
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/849
@bryancall Can you review please, you had some concerns on the previous PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/848#discussion_r74351692
--- Diff: plugins/regex_remap/README ---
@@ -53,11 +54,19 @@ to turn this off use the option 'no-query-string', e.g.
... @pparam=maps.r
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/848#discussion_r74351795
--- Diff: plugins/regex_remap/regex_remap.cc ---
@@ -759,15 +761,19 @@ TSRemapNewInstance(int argc, char *argv[], void **ih,
char * /* errbuf ATS_UNUSE
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/848#discussion_r74351870
--- Diff: plugins/regex_remap/regex_remap.cc ---
@@ -759,15 +761,19 @@ TSRemapNewInstance(int argc, char *argv[], void **ih,
char * /* errbuf ATS_UNUSE
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/848
Nicely done.
ð
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/848
Btw, one slight nit-pick (mostly for future reference): You should keep the
Subject line on the commit to < 50 characters, otherwise (as you can see)
Github does crazy line truncati
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/848
---
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/851
Ding dong, XML is dead? :-)
---
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/851
Fwiw, it's running on docs/ci, with the slightly modified squid log that we
use there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/852
[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, or
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/852
---
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/846
Should we land this?
---
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
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/851#discussion_r74513800
--- Diff: proxy/logging/LogConfig.cc ---
@@ -493,14 +494,21 @@ void
LogConfig::setup_log_objects()
{
Debug("log", "c
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/856
Clang-format.
---
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/855
[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, or
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/855
Needs to run "make clang-format". :)
---
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 th
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/858
TS-4184 Makes -std=c++11 mandatory
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4184
Alternatively you can
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/860
TS-4695 Removes the --enable-cppapi build option
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4695
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/861
TS-4360 Renames NPN_ defines to ALPN_
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4360
Alternatively you can
GitHub user zwoop opened a pull request:
https://github.com/apache/trafficserver/pull/862
TS-4691 Removes a bogus assertion which can cause failure
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-4691
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/862
I already did a while ago ( can't see the Jira right now). ð
---
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 pr
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/864
The safer of the two seems to be to honor the last seen header.
---
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/863
@oknet Can you take a look at this as well 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 a diff in the pull request:
https://github.com/apache/trafficserver/pull/866#discussion_r74707413
--- Diff: configure.ac ---
@@ -49,7 +49,7 @@ AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability
tar-ustar foreign no-installinf
AM_MAINTAINER_MODE
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/866#discussion_r74707669
--- Diff: proxy/logging/LogUtils.cc ---
@@ -359,6 +359,23 @@ LogUtils::escapify_url(Arena *arena, char *url, size_t
len_in, int *len_out, cha
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/866#discussion_r74707817
--- Diff: proxy/logging/LogUtils.cc ---
@@ -359,6 +359,23 @@ LogUtils::escapify_url(Arena *arena, char *url, size_t
len_in, int *len_out, cha
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/859
---
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 opened a pull request:
https://github.com/apache/trafficserver/pull/867
TS-2048 Avoids scheduling CFLUS compressor when not enabled
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zwoop/trafficserver TS-2048
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/855
Also would have a Jira # in the subject line 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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/870
:+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, or if the
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/868
---
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/867
---
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/861
---
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/862
---
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/858
---
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/860
Sigh, freebsd regression arbitrarily failed it seems ... try 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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/860
Rebooted the FreeBSD VM's 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 project does not have
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/855
Ah, so you have to create a Jira ticket (for now at least) in conjunction
with the Github PR. Go to
https://issues.apache.org/jira/browse/TS/?selectedTab=com.atlassian.jira.jira-projects
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/865
clang-format. Also, was this proposed on dev@ as well? I might have missed
it though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/866
@maskit Are you saying the patch as is is not good / sufficient?
---
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/863
@shinrich Lets do it! :)
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/869
I'm +1 on this, would be curious if the original Jira poster can / have
verify ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/771
Where are we with this? Ready to land?
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/769
@SolidWallOfCode Are we ready to land this?
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/568
Time to land this?
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/701
@oknet ping ?
---
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
I'm not sure if anyone has had the cycles to help debug this :-/ It might
be worth emailing dev@ with a concrete question / request for help.
---
If your project is set up for it, yo
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/772
@oknet For now, I think we should close this PR, wait for @alhonen to
clean up the underlying plumbing, and then revisit this? If you agree, please
close this PR for, but obviously keep your
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/871
Does this replace the previous commits from TS-3828? The last comment is a
little confusing, in that it sounds this would replace the previous commits?
---
If your project is set up for it
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/871
[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, or
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/873
Is it possible to break this up into individual PRs for each Jira? Or are
they interleaved? Also, please make the commit messages include each Jira in
the Summary, and also, we prefer one
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/874
[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, or
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/875
[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, or
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/873
Yeh, I think we should break this up before reviewing.
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/877
I have mixed feelings on the custom log for this. This is really for
intermittent testing of behavior, similar to e.g. the memory usage dump
feature. I.e. I can't imagine someone would use
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/872
Curious, where do these messages come from? Is that related to clustering?
[approveci]
---
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 a diff in the pull request:
https://github.com/apache/trafficserver/pull/880#discussion_r75419879
--- Diff: proxy/logging/LogAccessHttp.cc ---
@@ -769,8 +782,11 @@ LogAccessHttp::marshal_client_req_uuid(char *buf)
const char *uuid = (char
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/880#discussion_r75419974
--- Diff: proxy/logging/LogAccess.cc ---
@@ -326,11 +328,11 @@ LogAccess::marshal_proxy_resp_content_type(char *buf
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/882
Did we send an email to users@ making this announcement already? If not,
please do so, and specify that we've moved this functionality to the cachekey
plugin. [approve ci]
---
If your pr
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/877
[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, or
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/845
[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, or
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/877
Have to run clang format, (I highly recommend installing the git hook that
we have, see
https://github.com/apache/trafficserver/blob/master/tools/pre-commit
Also, I
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/871#discussion_r75571646
--- Diff: proxy/http/HttpTransact.cc ---
@@ -8059,12 +8059,8 @@ HttpTransact::build_response(State *s, HTTPHdr
*base_response, HTTPHdr *outgoing
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/860
---
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/884
Doing a quick git grep, I a few places that might need more cleanup?
proxy/logging/Log.h:SEND_NON_XML_CUSTOM_FMTS,
proxy/logging/Log.h:SEND_STD_AND_NON_XML_CUSTOM_FMTS
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/884
Also, there are several section in docs the refer to the XML format. I'm ok
with putting that on a different Jira issue though, and we can rope in @jsime
for help (lets just not forg
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/892
Ship it.
:+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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/891
I didn't test it, but seems very reasonable.
:+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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/890
Yeh, I like this, major drag we have to keep it for another full year. :-/.
:+1:
---
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/893
Seems this branch has merge conflicts, please rebase with current master
and push again before review.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/904#discussion_r75856806
--- Diff: lib/ts/InkErrno.cc ---
@@ -0,0 +1,106 @@
+/** @file
+
+ Error code defines
+
+ @section license License
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/904
:+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, or if the
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/902
Just to be picky, but we should avoid typos in the commit messages (memory,
not memory). I also think this could be simplified to just e.g.
4746 Fixes ParentRecord *secondary_parents
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/902
Also, I'm pretty sure this would fail the CI build, please run "make
clang-format" on the code, and push this again with the updated commit message.
Thanks!
---
If your
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/901
Looks good. Maybe I suggest a slightly cleaner commit message:
TS-4745 Initializes pRecord.failCount in ParentRecord::ProcessParents
:+1:
---
If your project is set up for it
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/900#discussion_r75857735
--- Diff: proxy/ParentConsistentHash.cc ---
@@ -143,6 +143,8 @@ ParentConsistentHash::selectParent(const
ParentSelectionPolicy *policy, bool fir
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/898
You might want to consider editing the commit subject, Github gets really
weird and truncates messages > 50 characters (so better do a short Summary, and
then more verbose messaging in the b
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/898
:+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, or if the
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/905
:+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, or if the
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/903
Few issues here:
1) There are merge conflicts
2) Please don't bundle more than one Jira into each PR (unless absolutely
necessary)
3) Squash commits to down t
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/895
I'm ok with this, but it's slightly misleading that we are changing both
the HTTP/1 and the H2 timeouts. Perhaps considering mentioning this in the
commit message body, or the Summ
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/897
I guess I don't have a strong opinion, but it seems unfortunate to me to
introduce these new options as headers, rather than URI components. The
original plugin for lighthttpd did everythi
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/882
Deprecated as of 6.2.0, removed for 7.0.0:
This plugin is deprecated as of v6.2.0 and will be removed as of v7.0.0. It
is replaced by a new Cache Key Manipulation Plugin and you should
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/879
@jpeach we good to land this now?
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/893
[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, or
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/907
:+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, or if the
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/897
I dislike the inconsistency, but your explanation / use case makes a lot of
sense. And if we agree that this makes things better, and not worse, do it.
Maybe file a Jira to have a future
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/912
I applied the patch, and could not see any remnants of _pct metrics.
:+1:
---
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/918
Be careful when eliminating try_to_expand_host_name(), I recently
discovered that this function also deals with DNS failures in parent selection.
I'm +1 on removing (or renaming) this m
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/923
Fwiw, we should make sure to review and mark a PR for CI build before
merging.
---
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/928
Also update the commit message :)
---
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
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/936
:+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, or if the
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/931
It's a shame, because this plugin works poorly (only one request is
"saved").
---
If your project is set up for it, you can reply to this email and have your
reply appear on Git
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/929
Fwiw, if the Linux build succeeds, it means clang-format succeeded.
---
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/928
Fwiw, Github sort of imposes a Subject length of 50 characters, it will
line wrap beyond that, making for ugly messages. So, I think we should recommend
TS-1234 < 50 charac
301 - 400 of 910 matches
Mail list logo