Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Configure session tracking on TO side#754

Closed
weifensh wants to merge 2 commits into
apache:masterfrom
weifensh:master-session-tracking-ops-4
Closed

Configure session tracking on TO side#754
weifensh wants to merge 2 commits into
apache:masterfrom
weifensh:master-session-tracking-ops-4

Conversation

@weifensh

Copy link
Copy Markdown
Contributor

This PR work together with PR #643

@limited

limited commented Jul 26, 2017

Copy link
Copy Markdown
Contributor

Thanks John-
The diff is quite large, mostly because of whitespace and some formatting related changes.

Can you please try to minimize the size of the diff to just changes directly related to the Session Tracking code so that its easier to review?

@weifensh

Copy link
Copy Markdown
Contributor Author

@limited Thanks Eric. The white spaces and tabs are mostly introduced by perltidy. Some are caused by reformatting with the new code. And others are because it appears some original files are not following the formatting conventions at https://trafficcontrol.incubator.apache.org/docs/latest/development/traffic_ops.html and thus they are reformatted by perltidy in this PR. I might be asked to run perltidy if I reduce the diff;-)

@asfgit

asfgit commented Aug 6, 2017

Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

2 similar comments
@asfgit

asfgit commented Aug 6, 2017

Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@asfgit

asfgit commented Aug 14, 2017

Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@asfgit

asfgit commented Aug 15, 2017

Copy link
Copy Markdown
Contributor

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR-trafficops-test/14/

@dewrich

dewrich commented Aug 16, 2017

Copy link
Copy Markdown
Contributor

@limited @weifensh Is there a way we can prevent adding yet another field to the Delivery Service Table? If we are tracking sessions, maybe that should be a separate table (with the session metadata we care about) that points back to the Delivery Service table or even the User table if that is required.

Also, where is the JIRA for this change?

@mitchell852

Copy link
Copy Markdown
Member

and was this discussed on the ML because i'm a pretty hard -1 on more DS table columns (if possible). That table is way out of control imo (50+ columns already?). @knutsel did introduce ds profiles...maybe that could be leveraged instead? i don't know.

@limited

limited commented Aug 16, 2017

Copy link
Copy Markdown
Contributor

There's 2 fields- a boolean and a list of the keys to use from the cookie. The list is expected to be short, just a few entries long. No session metadata is stored in the TO DB.

I agree we need better approach to DS options, but I don't think this is the PR to hinge things on.

@mitchell852 mitchell852 added the Traffic Ops related to Traffic Ops label Oct 5, 2017
@anna-k-1

Copy link
Copy Markdown
Contributor

@weifensh We are going to close this - sorry! If you still want this feature, please rewrite in Go and resubmit and someone will check it out.

@rob05c rob05c closed this Sep 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants