[
https://issues.apache.org/jira/browse/PHOENIX-4750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16573262#comment-16573262
]
Alex Araujo edited comment on PHOENIX-4750 at 8/8/18 2:01 PM:
--------------------------------------------------------------
v3 addresses most of your requests.
{code:java}
+ private static final ServerCustomizersFactory DEFAULT_SERVER_CUSTOMIZERS =
+ new ServerCustomizersFactory.ServerCustomizersFactoryImpl();{code}
{quote}Move this to the top of the Class.
{quote}
I placed it similar to DEFAULT_USER_EXTRACTOR. Move both in a follow up?
{code:java}
// Start building the Avatica HttpServer
- final HttpServer.Builder builder = new
HttpServer.Builder().withPort(port)
- .withHandler(service, getSerialization(getConf()));
+ final HttpServer.Builder<Server>
+ builder =
+ HttpServer.Builder.<Server>newBuilder().withPort(port)
+ .withHandler(service, getSerialization(getConf()));{code}
{quote}Formatting on this is pretty weird. Can you make is closer to what it
was originally?
{quote}
Nice catch. Ran the formatter on those lines.
{code:java}
@VisibleForTesting
+ public void enableServerCustomizersIfNecessary(HttpServer.Builder<Server>
builder, Configuration conf) {{code}
{quote}I don't see this method used anywhere in your patch. Is it actually used
by tests?
{quote}
{{It's used in QueryServer#run(String[]) right after:}}
{{setRemoteUserExtractorIfNecessary(builder, getConf());}}
Meant to also add a unit test for it in the first round; added it in v3 along
with the new test you suggested.
Thanks for taking another look [~elserj].
Edit: Fixed formatting
was (Author: alexaraujo):
v3 addresses most of your requests.
{code:java}
+ private static final ServerCustomizersFactory DEFAULT_SERVER_CUSTOMIZERS =
+ new ServerCustomizersFactory.ServerCustomizersFactoryImpl();{code}
{quote}Move this to the top of the Class.
{quote}
I placed it similar to DEFAULT_USER_EXTRACTOR. Move both in a follow up?
// Start building the Avatica HttpServer- final HttpServer.Builder
builder = new HttpServer.Builder().withPort(port)
- .withHandler(service, getSerialization(getConf()));
+ final HttpServer.Builder<Server>
+ builder =
+ HttpServer.Builder.<Server>newBuilder().withPort(port)
+ .withHandler(service, getSerialization(getConf()));
{quote}Formatting on this is pretty weird. Can you make is closer to what it
was originally?
{quote}
Nice catch. Ran the formatter on those lines.
{code:java}
@VisibleForTesting
+ public void enableServerCustomizersIfNecessary(HttpServer.Builder<Server>
builder, Configuration conf) {{code}
{quote}I don't see this method used anywhere in your patch. Is it actually used
by tests?
{quote}
{{It's used in QueryServer#run(String[]) right after:}}
{{setRemoteUserExtractorIfNecessary(builder, getConf());}}
Meant to also add a unit test for it in the first round; added it in v3 along
with the new test you suggested.
Thanks for taking another look [~elserj].
> Resolve server customizers and provide them to Avatica
> ------------------------------------------------------
>
> Key: PHOENIX-4750
> URL: https://issues.apache.org/jira/browse/PHOENIX-4750
> Project: Phoenix
> Issue Type: Improvement
> Reporter: Alex Araujo
> Assignee: Alex Araujo
> Priority: Major
> Fix For: 4.15.0
>
> Attachments: PHOENIX-4750.patch, PHOENIX-4750.v2.patch,
> PHOENIX-4750.v3.patch
>
>
> CALCITE-2284 allows finer grained customization of the underlying Avatica
> HttpServer.
> Resolve server customizers on the PQS classpath and provide them to the
> HttpServer builder.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
