CurtHagenlocher commented on issue #4231:
URL: https://github.com/apache/arrow-adbc/issues/4231#issuecomment-4261367803
I started to rework the details and it reminded me of some of the reasons
I'd picked `AdbcError`. Semantically, it's like returning a `401` from an HTTP
request: "this request was an error, but if you retry with new credentials it
might work". It also happens that the `AdbcErrorDetail` API is a cheap and
flexible way to pass arbitrary key-value pairs, and it would be annoying to
create another instance of basically the same thing.
As far as updating the credential property or properties, I agree that
calling `SetOption` on the connection might require extra complexity in the
driver to deal with reentrancy. But nearly any other approach is going to have
a heavier API footprint; at a minimum, I'm going to need to be able to return
at least one value which means that someone will have to free that buffer
afterwards. The simplest way to avoid this is to reuse an existing structure
which already accommodates cleanup. Here too, `AdbcError` is a possibility --
but one that's even more dubious than the previous use I suggested. Another
alternative would be to treat this like a `Bind` and have the host populate an
`ArrowArray` and `ArrowSchema` with the details.
(Side note: I'm curious now why `AdbcErrorDetail` wasn't implemented in
terms of `ArrowArray` and `ArrowSchema`. This would be more generic because it
would incorporate explicitly-typed data whereas -- AFAICT -- `AdbcErrorDetail`
types have to be inferred by the host.)
Anyhow, I iterated a bit on this idea and came up with
```
typedef AdbcStatusCode (*AdbcCredentialHandler)(struct AdbcError* details,
struct ArrowSchema* schema, struct
ArrowArray* data,
void* user_data);
AdbcStatusCode (*ConnectionSetCredentialHandler)(struct AdbcConnection*,
AdbcCredentialHandler
handler,
void* user_data, struct
AdbcError*);
```
I don't know how we feel about in/out arguments, but the conceptual idea is
this:
The driver gets an error from the data source that it identifies as a
possible credential expiration error. It populates the error details as an
`AdbcError` and then puts the relevant credential-related parameters into a
single-row record batch. It passes these to the callback. One of three things
happens:
1) The callback looks at the error and other properties and decides whether
or not it can refresh the credential. If it can't, then it returns
`ADBC_STATUS_CANCELLED` to show that it did not attempt a credential refresh.
It doesn't touch the other parameters. The driver releases all the data
structures when control returns to it.
2) The callback tries to refresh the credential but the refresh fails. The
callback returns `ADBC_STATUS_UNAUTHENTICATED` to show this. It may optionally
release the error `details` and populate it with new information about the
credential refresh failure. Again, the driver releases all the data structures
when control returns to it.
3) The refresh succeeds. The callback releases `data` and populates the new
credential values in that array. It returns `ADBC_STATUS_OK`. The driver reads
the new credential properties from the `data` and then releases all the data
structures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
