Dan Hecht has posted comments on this change.
Change subject: Preview: Re-enable Kudu in build using client stubs when needed
......................................................................
Patch Set 1:
> > Before combing through the details, I'd like to make sure we're
> on
> > the same page about where we're going with this. My understanding
> > from our discussion on Friday was that we were going to have
> client
> > stubs (TBD how that is created) which allow us to avoid
> conditional
> > compilation or runtime feature switches. E.g. if you try to do
> > something w/ Kudu on an unsupported operating system the query
> > would return a reasonable failure. Is that what you had in mind
> as
> > well? (I think the right thing to do by the time we release is to
> > have analysis reject these queries before hitting the backend,
> but
> > we can talk about that later if the client stubs are smart enough
> > to return errors.) If so, can you describe the plan to getting
> > there from here which still has compilation and runtime flags?
>
> Yes that sounds right.
>
> The compilation differences are very minimal now. If the client
> were somehow able to indicate if it's suitable for use, the
> compilation could be the same. The changes to this patch could be
> as small as changing the implemenation of KuduIsAvailable() to call
> the client method that indicates if the client should actually be
> used.
>
> The runtime flag is more of a convenience. It allows simulating
> having the non-working client. If the client itself were able to
> indicate that, then the flag could be removed.
I think we should get the compilation difference to zero.
Maybe one option is to declare a weak symbol, say KuduClientStub) which is
defined only by the stubbed client (but not the actual client). Then
KuduIsAvailable() { return KuduClientStub == NULL; }
--
To view, visit http://gerrit.cloudera.org:8080/2585
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bf1c964faf21722137adc4f7ba7f78654f0f712
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Casey Ching <[email protected]>
Gerrit-Reviewer: Casey Ching <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-HasComments: No
