Add notes/comments field to parameters

[mitchell852]

This Improvement request (usability, performance, tech debt, etc.) affects these Traffic Control components:

• Traffic Ops
• Traffic Portal

I believe this would just be a TO/TP change. It would be a new field on the TO parameter object that t3c could ignore. The field is for humans.

Current behavior:

Parameters have 4 fields:

1. name
2. value
3. config_file
4. secure (boolean)

there is no field to add comments or notes to explain why a particular value was chosen for a parameter leaving operators to guess why the value was chosen.

New behavior:

Add a 5th field (comments/notes) where humans can add comments regarding the parameter. i.e. why a particular value was chosen.

[ntheanh201]

I did a PR on this issue, please take a look!

[mitchell852]

@ntheanh201 - can you provide a link to the PR?

[ntheanh201]

@ntheanh201 - can you provide a link to the PR?

it's here: https://github.com/apache/trafficcontrol/pull/7845

[zrhoffman]

Responding to https://github.com/apache/trafficcontrol/pull/7845#issuecomment-1789380343 here.

I am -1 on this idea. The promise of a comments field being used only for humans' benefits has been repeatedly broken on other objects, and a Parameter ought to be too small to express anything complex enough to require a comment.

We provide only features. We don't get to decide how users use those features. In your words:

These are project-level decisions. Sometimes adding new features, addressing newly discovered concerns, or meeting new constraints absolutely requires breaking changes. It's great for the Go project itself that the decision was made to never make breaking changes at the source level, but this is not the reality of the entire world of software development. In my honest opinion, a language so restrictive that beyond a certain point in working with it on a project no further changes of a certain nature can be made is fundamentally broken.

So trying to police how people use ATC hurts ATC and we shouldn't do it. I already know of one company that does not contribute their many ATC additions back to the proect, because their use case for ATC lies outside the scope of changes that ATC typically accepts. They are probably the most successful user of ATC, and not accepting unconventional changes is our loss.

In fact, Parameters as objects should not even exist in their own right, IMO, and - especially now that Profile layering is possible - instead just be properties of Profiles.

You have already voiced your opinion about how you think things things shoul be organized in your Global Configuration blueprint (#7015), and blocking other people's incremental PRs because you think a different high-effort idea is better hurts the project.

In some respects, they are represented that way already, and that means that this amounts to, in those cases, having an unbounded number of free-form text fields on a collection of free-form text fields on a Profile.

The author of #7845 clearly wants this feature, as do several other ATC users who I have talked to. It sounds like you're against implementing #7456 because of the edge case of a user potentially trying to derive structure out of the comments, but that's not a good enough to block accepting implementation itself.

If you're concerned about an edge case like that, you could suggest on the PR adding a note to the field that it should only be used to document the property's purpose, and that using it for a structural reason would be unsound.

[ocket8888]

We provide only features. We don't get to decide how users use those features. In your words:

These are project-level decisions. Sometimes adding new features, addressing newly discovered concerns, or meeting new constraints absolutely requires breaking changes. It's great for the Go project itself that the decision was made to never make breaking changes at the source level, but this is not the reality of the entire world of software development. In my honest opinion, a language so restrictive that beyond a certain point in working with it on a project no further changes of a certain nature can be made is fundamentally broken.

I'm not sure what you're saying. This is a new feature is it not? I'm not objecting on the grounds that it's a breaking change; that would be silly. I've authored many a breaking change.

You have already voiced your opinion about how you think things things shoul be organized in your Global Configuration blueprint (https://github.com/apache/trafficcontrol/pull/7015), and blocking other people's incremental PRs because you think a different high-effort idea is better hurts the project.

That's not at all what's happening here. I'm against this idea because I think it would have a negative impact on its own terms. Not because I have some superior idea. If I'd never given Global Configuration a moment's thought, I would still think that comments on Parameters would be a bad idea - which is also kind of a moot point, because I'm not actually sure how those two ideas are related. Global Configuration would remove some dedicated Parameters, they would do nothing to the notion of Parameters overall. I don't see how those are conflicting ideas.

The author of https://github.com/apache/trafficcontrol/pull/7845 clearly wants this feature, as do several other ATC users who I have talked to. It sounds like you're against implementing https://github.com/apache/trafficcontrol/issues/7456 because of the edge case of a user potentially trying to derive structure out of the comments, but that's not a good enough to block accepting implementation itself.

I had guessed that the author of that PR opened it only because this issue exists. He or she has said nothing to indicate any personal attachment to the idea. I also can't speak to the desires of others you've talked to, because they haven't taken part in the discussion.

Which is what this is, ultimately. I'd be more than happy to take this to the mailing list. But this is kind of totally standard procedure. Somebody proposed a change, I have concerns that it would ultimately be a net negative impact, and I voiced them. Then nobody else addressed - or even acknowledged - those concerns. So to somehow accuse me of standing in the way of something everyone wants is pretty strange to me. Requiring consensus on changes to the project isn't my dastardly plan to impede some specific PR, it's a founding principle of the ASF and older by far than this project. I'm just taking part, is all.

[zrhoffman]

I had guessed that the author of that PR opened it only because this issue exists.

I could be wrong, but from the implementer's contribution history, I got the idea that they are using ATC in some capacity. So it stands to reason that, if they opened a PR, they would find practical benefit to the feature being implemented. In some cases, though maybe not this one, a contributor is even using the feature they are PRing before it is even merged.

Which is what this is, ultimately. I'd be more than happy to take this to the mailing list.

Sure

But this is kind of totally standard procedure. Somebody proposed a change, I have concerns that it would ultimately be a net negative impact, and I voiced them. Then nobody else addressed - or even acknowledged - those concerns. So to somehow accuse me of standing in the way of something everyone wants is pretty strange to me.

To be clear, I don't take issue with anything you've said, only that you -1ed the Issue on a contributor's PR.

Requiring consensus on changes to the project isn't my dastardly plan to impede some specific PR, it's a founding principle of the ASF and older by far than this project. I'm just taking part, is all.

Agreed, but IMO special care should be taken in cases when -1ing an Issue only after a contributor has submitted a PR for it, and that includes not -1ing the Issue on that PR. The will of users of ATC to give back to the project is often fragile and contributors are easily dissuaded, so we need to do what we can to instead encourage their contributions, even in cases where discussion on an Issue means that a contributor's corresponding PR is not likely to be accepted.

[ntheanh201]

Actually, I'm (we’re) using ATC to develop a CDN. I took this issue to implement because I see that we need it, we need more context of each parameter’s objective and why we need it. It's not a kinda high impact issue but IMO it's nice to have. So that's why I contribute back to the community, and I also wanna become a long-term contributor to this project

[ocket8888]

Actually, I'm (we’re) using ATC to develop a CDN.

Thank you for clarifying.

... I also wanna become a long-term contributor to this project

You're well on your way.

[zrhoffman]

@ocket8888: Knowing that @ntheanh201 is a CDN developer who has has a practical use case for #7456, do you think #7845 is okay as-is? Or does it need adusting first?

[ocket8888]

I still think comments on Parameters is a sign that they are a bloated concept. The biggest issue with that being that if we ever decide to absorb some parameters into properly validated Delivery Service and/or Server and/or Profile properties, any information stored as a comment would be destroyed.

If we're cool with that so that forward progress on eliminating unnecessarily complex parameters can proceed anyway, then i dont want to stand in the way of someone adding something useful to them.

[zrhoffman]

I still think comments on Parameters is a sign that they are a bloated concept.

Agreed, it makes up for a shortcoming in parameters. But this is not a shortcoming of the comment field itself

The biggest issue with that being that if we ever decide to absorb some parameters into properly validated Delivery Service and/or Server and/or Profile properties, any information stored as a comment would be destroyed.

Hopefully, the purpose of those particular parameters is well-documented at that point that removing their comments does not cause confusion.