Skip to content

Conversation

maskit
Copy link
Member

@maskit maskit commented Oct 9, 2025

Apparently, client netvc is not always available on READ_REQUEST_HDR_HOOK.

@maskit maskit added this to the 10.2.0 milestone Oct 9, 2025
@maskit maskit self-assigned this Oct 9, 2025
Copy link
Contributor

@brbzull0 brbzull0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.

Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, approving as is. But would a debug log for the else case be helpful potentially? If you want to add that, I can give you a quick approval after another commit. Otherwise feel free to merge with these changes as is.

@bryancall
Copy link
Contributor

I would like to see the implementation being safer by doing null check on the input arguments and return TS_ERROR.

@maskit
Copy link
Member Author

maskit commented Oct 14, 2025

Looks fine, approving as is. But would a debug log for the else case be helpful potentially?

There's a debug log on the caller side, and that works for all address sources.

I would like to see the implementation being safer by doing null check on the input arguments and return TS_ERROR.

I do not think that is a good idea. If we took that way, null check would be done again and again by all functions that receive vconn on every call. So redundant.

@maskit maskit merged commit b187778 into apache:master Oct 14, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants