phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lev Bronshtein (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (PHOENIX-4808) Phoenix Spark Do not verify zKUrl in DefaultSource.scala
Date Wed, 11 Jul 2018 18:13:00 GMT

     [ https://issues.apache.org/jira/browse/PHOENIX-4808?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Lev Bronshtein updated PHOENIX-4808:
------------------------------------
    Description: 
Currently verifyParameters will throw an exception

_[zkUrl check|https://github.com/apache/phoenix/blob/master/phoenix-spark/src/main/scala/org/apache/phoenix/spark/DefaultSource.scala#L56]_

_if (parameters.get("zkUrl").isEmpty)_ however this is unnecessary as ultimately new PhoenixRelation(...)

[PhoenixRelation|https://github.com/apache/phoenix/blob/master/phoenix-spark/src/main/scala/org/apache/phoenix/spark/DefaultSource.scala#L29]

is called (see PhoenixRelation.scala) which in turn calls new PhoenixRDD(...) (See PhoenixRDD.scala). 
PhoenixRDD constructor declares zkUrl as optional.  PhoenixRDD will try to use zkUrl and
fall back on HBase Configuration

 
{code:java}
// Override the Zookeeper URL if present. Throw exception if no address given.
zkUrl match {
    case Some(url) => ConfigurationUtil.setZookeeperURL(config, url)
    case _ => {
        if(ConfigurationUtil.getZookeeperURL(config).isEmpty) {
            throw new UnsupportedOperationException(
              s"One of zkUrl or '${HConstants.ZOOKEEPER_QUORUM}' config property must be provided"
        )
    }
}
{code}
 

I propose that this check is unnecessary and should be removed.  As we will still get an
exception, but a little later

  was:
Currently verifyParameters will throw an exception

_[zkUrl check|https://github.com/apache/phoenix/blob/master/phoenix-spark/src/main/scala/org/apache/phoenix/spark/DefaultSource.scala#L56]_

_if (parameters.get("zkUrl").isEmpty)_ however this is unnecessary as ultimately new PhoenixRelation(...)

[PhoenixRelation|https://github.com/apache/phoenix/blob/master/phoenix-spark/src/main/scala/org/apache/phoenix/spark/DefaultSource.scala#L29]

is called (see PhoenixRelation.scala) which in turn calls new PhoenixRDD(...) (See PhoenixRDD.scala). 
PhoenixRDD constructor declares zkUrl as optional.  PhoenixRDD will try to use zkUrl and
fall back on HBase Configuration

 
{code:java}
// Override the Zookeeper URL if present. Throw exception if no address given.
zkUrl match {
case Some(url) => ConfigurationUtil.setZookeeperURL(config, url)
case _ => {
if(ConfigurationUtil.getZookeeperURL(config).isEmpty) {
throw new UnsupportedOperationException(
s"One of zkUrl or '${HConstants.ZOOKEEPER_QUORUM}' config property must be provided"
)
}
}
{code}
 

I propose that this check is unnecessary and should be removed.  As we will still get an
exception, but a little later


> Phoenix Spark Do not verify zKUrl in DefaultSource.scala
> --------------------------------------------------------
>
>                 Key: PHOENIX-4808
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-4808
>             Project: Phoenix
>          Issue Type: Improvement
>            Reporter: Lev Bronshtein
>            Priority: Major
>
> Currently verifyParameters will throw an exception
> _[zkUrl check|https://github.com/apache/phoenix/blob/master/phoenix-spark/src/main/scala/org/apache/phoenix/spark/DefaultSource.scala#L56]_
> _if (parameters.get("zkUrl").isEmpty)_ however this is unnecessary as ultimately new
PhoenixRelation(...)
> [PhoenixRelation|https://github.com/apache/phoenix/blob/master/phoenix-spark/src/main/scala/org/apache/phoenix/spark/DefaultSource.scala#L29]
> is called (see PhoenixRelation.scala) which in turn calls new PhoenixRDD(...) (See PhoenixRDD.scala). 
PhoenixRDD constructor declares zkUrl as optional.  PhoenixRDD will try to use zkUrl and
fall back on HBase Configuration
>  
> {code:java}
> // Override the Zookeeper URL if present. Throw exception if no address given.
> zkUrl match {
>     case Some(url) => ConfigurationUtil.setZookeeperURL(config, url)
>     case _ => {
>         if(ConfigurationUtil.getZookeeperURL(config).isEmpty) {
>             throw new UnsupportedOperationException(
>               s"One of zkUrl or '${HConstants.ZOOKEEPER_QUORUM}' config property must
be provided"
>         )
>     }
> }
> {code}
>  
> I propose that this check is unnecessary and should be removed.  As we will still get
an exception, but a little later



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message