Only Check includeEndStreamAction in response header when needed #591
Conversation
| @@ -336,7 +336,7 @@ class DeltaSharingRestClient( | |||
| val target = getTargetUrl( | |||
| s"/shares/$encodedShareName/schemas/$encodedSchemaName/tables/$encodedTableName/metadata" + | |||
| s"$encodedParams") | |||
| val response = getNDJson(target) | |||
| val response = getNDJson(target, requireVersion = true, checkEndStreamActionHeader = false) | |||
There was a problem hiding this comment.
Question: do we set includeendstreamaction for metadata queries?
If do, would it be a problem if some DS server returns endStreamAction for it?
There was a problem hiding this comment.
makes sense, updated to not set includeendstreamaction for table version and metadata queries.
| @@ -495,7 +500,9 @@ class DeltaSharingRestClient( | |||
| val (version, respondedFormat, lines, _) = getFilesByPage(table, target, request) | |||
| (version, respondedFormat, lines) | |||
| } else { | |||
| val response = getNDJson(target, request) | |||
| val response = getNDJsonPost( | |||
| target = target, data = request, setIncludeEndStreamAction = true | |||
There was a problem hiding this comment.
(optional, for my readability)
consider renaming: includeEndStreamAction -> endStreamActionEnabled
We can then setIncludeEndStreamAction = endStreamActionEnabled where we are setting it to true, and then at other places only test for setIncludeEndStreamAction.
There was a problem hiding this comment.
do you mean the renaming of the header or variable name?
There was a problem hiding this comment.
The variable name @linzhou-db
| @@ -871,14 +871,10 @@ class DeltaSharingRestClient( | |||
| 0L | |||
| } | |||
| }, | |||
| respondedFormat = respondedFormat, | |||
| includeEndStreamActionHeader = includeEndStreamActionHeader, | |||
| respondedFormat = getRespondedFormat(capabilities), | |||
There was a problem hiding this comment.
Optional optimization:
Can getResponse itself create the capabilitesMap to avoid duplicate parsing and map creation from capabilities, since it seems the capabilities map is what is used everywhere?
Only Check includeEndStreamAction in response header when needed: