-
Notifications
You must be signed in to change notification settings - Fork 151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support delta-sharing-capabilities header in DeltaSharingRestClient #338
Conversation
@wchau Can you please also look at this change? |
val (version, lines) = getNDJson(target) | ||
val (version, respondedFormat, lines) = getNDJson(target) | ||
if (responseFormat != respondedFormat) { | ||
logWarning(s"RespondedFormat($respondedFormat) is different from requested responseFormat(" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps add a comment that this can happen if the DS server does not support the requested format.
Ideally, this should only happen if we request delta format but get back parquet. The reverse is not expected right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right!
And this happens on the two recipient on uc use cases, where the delta sharing server is an open servers, and recipients are on databricks DBR, so recipients would any way use this code to ask for "delta" format, but may get parquet in response. And we should still be able to handle that case.
Support delta-sharing-capabilities header in DeltaSharingRestClient, if
responseFormat=delta
, then we don't parse the returned lines, just return them as a Seq[String]. In the tests, we are still trying to check the returned fields by directly matching the string.For tests on exceptions, we use
DeltaSharingRestClientSuite
as the responseFormat shouldn't matter. For tests with successfully returned rpcs, we add a newDeltaSharingRestClientDeltaSuite
to check the returned lines.Please only review the changes under
client/
in this PR, changes underserver/
should be reviewed with #335.This is part 2 for issue #341
TODO: check whether what we should do with random capabilities header, error or ignore.