Deprecate TSSslSecretGet (not thread safe).

[ywkaras]

The current version is not thread safe because it returns a pointer into a mutex-protected data structure, which may be invalidated by another thread

[bneradt]

Looks good. It's good to give a heads-up to people that this function signature will be changing in 10.x.

I suggest providing a brief description of the new signature now in these comments.

[ywkaras]

The description is available in the Rel 10 documentation though.

[bneradt]

The description is available in the Rel 10 documentation though.

That's a good point. I would point to that doc, then. Something like:

See the 10.x release documents for a description of the new signature which will replace this one.

My intention is to provide the user with something actionable. Usually deprecation statements say, "This is the old interface and will only be supported for a limited amount of time. We encourage you to move to this new API in preparation for that." In this case, the new API will only come in the future release when this one is replaced, which is fine. But at least we can point the user to what the new API will be so they can prepare for it.

But that's just my two cents, and maybe misplaced. I haven't "requested changes" intentionally. Someone else will have to approve anyway since I'm also with Yahoo.

Thanks!

[maskit]

Do we deprecate things on minor releases? I feel like we've been doing it only on major releases but I'm not sure. If it's deprecated on 9.2.0, what's the alternative way that is not deprecated on the version?

[bneradt]

The description is available in the Rel 10 documentation though.

Do we deprecate things on minor releases? I feel like we've been doing it only on major releases but I'm not sure. If it's deprecated on 9.2.0, what's the alternative way that is not deprecated on the version?

Right, most deprecation warnings point people to an updated API to use. This is a different type of deprecation warning. In this case, we're fixing a race condition with TSSslSecretGet, but we have to change the signature to do so. That is, the function name will be the same, but the return type and some of the arguments will change. Such a change is backwards incompatible. For this reason, we're putting off the fix till 10.x. Walt is alerting people of this deprecation with this PR.

My suggestion to point to the new 10.x documentation with the updated signature is my attempt to clarify this.

[maskit]

Is the change #8790? It isn't even approved. Do we have consensus on the way of depreciation, which only changes the return type and the arguments?

I also found that TSSslSecretGet was introduced by #6609 and its milestone is 9.2.0, which means the API haven't been released publicly. If it's true, I think we can just remove the API.

[ywkaras]

Is the change #8790? It isn't even approved. Do we have consensus on the way of depreciation, which only changes the return type and the arguments?

I also found that TSSslSecretGet was introduced by #6609 and its milestone is 9.2.0, which means the API haven't been released publicly. If it's true, I think we can just remove the API.

I was told not to mark #8790 for back port to 9.2. If #8790 is rejected, we will have to eliminate or replace TSSslSecretGet() in another PR, it is not thread safe. #8790 is currently running in Yahoo prod, I was waiting for it to soak for a while before trying to merge it in open source.

[maskit]

I was told not to mark https://github.com/apache/trafficserver/pull/8790 for back port to 9.2

I wonder why. Backporting it to 9.2 is another option indeed.

[ywkaras]

I was told not to mark #8790 for back port to 9.2

I wonder why. Backporting it to 9.2 is another option indeed.

@bryancall do we want to reconsider back porting?

[ywkaras]

This won't be needed if https://github.com/apache/trafficserver/pull/8854 makes it into 9.2.

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.