GH-139: [Flight] Stop return null from MetadataAdapter.getAll(String) and getAllByte(String)#1016
GH-139: [Flight] Stop return null from MetadataAdapter.getAll(String) and getAllByte(String)#1016axreldable wants to merge 2 commits intoapache:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
|
Please add the |
ennuite
left a comment
There was a problem hiding this comment.
Hello @axreldable
This looks good to me, thank you for it.
I note that this also fixes an NPE in MetadataAdapter.getAllByte(String key)
Correct, it fixes NPE for |
…tring) and getAllByte(String)
0bbd70d to
dbd8b08
Compare
jbonofre
left a comment
There was a problem hiding this comment.
Overall LGTM.
Can you just clarify the case when it is null in the ServerSessionMiddleware please ?
flight/flight-core/src/main/java/org/apache/arrow/flight/client/ClientCookieMiddleware.java
Show resolved
Hide resolved
flight/flight-core/src/main/java/org/apache/arrow/flight/ServerSessionMiddleware.java
Show resolved
Hide resolved
|
@jbonofre , some CI pipelines failed. Do you think it relates to the changes or the pipelines are flaky? |
|
@axreldable let me take a look. I will get back to you. |
|
The verify job failed to flaky test (memory allocation). I ran again the jobs to double check. |
What's Changed
CallHeadershas 3 implementations:Before this change:
MetadataAdaptercould returnnullfromgetAll(String)andgetAllByte(String)when there were no values for the key, because gRPC’sMetadata.getAll()returnsnullin that case. This was undocumented and forced callers to null-check.After this change:
All 3 implementations consistently return an
empty iterable(nevernull) when the key is absent or has no values. The contract is documented on the interface and covered by tests for each implementation.This contains breaking changes.
MetadataAdapter.getAll(String)andgetAllByte(String)return empty iterator instead of null.Closes #139.