> On Jan. 26, 2017, 5:13 a.m., Rohini Palaniswamy wrote:
> > core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java,
> >  line 427
> > <https://reviews.apache.org/r/55963/diff/1/?file=1615995#file1615995line427>
> >
> >     Just use getHostName(). Normalization is unrelated to getting canonical 
> > host name. You can refer to uri.normalize() to understand what normalize 
> > does.

We have been using normalizeHostname in oozie few places, like 
ProxyUserService.normalizeHostname().

Hive also same namining convention few places. 
hive/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/ProxyUserSupport.java

getHostName() can be a confusing name, as it doesn't indicate what are we 
trying to get? Why do we need to get hostname for a hostname?


> On Jan. 26, 2017, 5:13 a.m., Rohini Palaniswamy wrote:
> > core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java,
> >  lines 436-437
> > <https://reviews.apache.org/r/55963/diff/1/?file=1615995#file1615995line436>
> >
> >     This host/port splitting is unnecessary. Add a getHost method to 
> > HCatURI and use that to just get uri.getHost(). In fact you can add 
> > HCatURI.getCanonicalHost() method and use that as well.
> >     
> >     i.e
> >     
> >     String getHostName(HCatURI hcatURI) {
> >        if (useCanonicalHostName) {
> >           return hcatURI.getCanonicalHost();
> >        } else {
> >           return hcatURI.getHost();
> >        }
> >     }

We have to split and/or join.
Map key is host+ port + "DELIMITER + hcatURI.getDb() + DELIMITER + 
hcatURI.getTable();

Even if we get hostname from HCatURI, we have to append port to form map key.
That will also mandate that all functions of HCatDependencyCache should use 
HCatURI as input. Currently, some of the function doesn't take HCatURi.
like     

public Collection<String> markDependencyAvailable(String server, String db, 
String table,
            Map<String, String> partitions) {


> On Jan. 26, 2017, 5:13 a.m., Rohini Palaniswamy wrote:
> > core/src/main/resources/oozie-default.xml, line 216
> > <https://reviews.apache.org/r/55963/diff/1/?file=1615996#file1615996line216>
> >
> >     We don't use camel case in property names.
> >     
> >     oozie.service.HCatAccessorService.cache.use.canonical.hostname
> 
> Rohini Palaniswamy wrote:
>     Actually oozie.service.HCatAccessorService.jms.use.canonical.hostname 
> would be better.

We do use camel case for property. Agree that 
oozie.service.HCatAccessorService.jms.use.canonical.hostname is a better name.


- Purshotam


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55963/#review163095
-----------------------------------------------------------


On Jan. 25, 2017, 11:52 p.m., Purshotam Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55963/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2017, 11:52 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2781
>     https://issues.apache.org/jira/browse/OOZIE-2781
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> OOZIE-2781 HCat partition available notification is not sent to coordinator 
> actions if coordinator job is using a different hostname (cname, IP address, 
> etc. ) for HCat URL.
> 
> 
> Diffs
> -----
> 
>   
> core/src/main/java/org/apache/oozie/dependency/hcat/EhcacheHCatDependencyCache.java
>  3bc467535202e13387b1d29ac678573f4154c522 
>   
> core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
>  9e24c9aa5f10821bf76029c51bb49e0728376caa 
>   core/src/main/resources/oozie-default.xml 
> ad103864d7631e9d3678a828bf297ca1c80f09ca 
>   
> core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java
>  a5d2ed92ffe9db77e2c8914b66c7806275dcc592 
> 
> Diff: https://reviews.apache.org/r/55963/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Reply via email to