> 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. > > Purshotam Shah wrote: > 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?
I believe hcat code for proxy users was mostly copied from Oozie code. I would still prefer canonicalizeHostname() because that is the correct terminology. Replacing IP address with domain name is usually a part of normalization but canonicalization is the specific terminology for this case. https://en.wikipedia.org/wiki/URL_normalization Not to be confused with URL canonicalization. If you still prefer to use normalize to be same as ProxyUserService, either change it to normalizeHostName() or getNormalizedHostName() instead of getNormalizeHostName() > 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(); > > } > > } > > Purshotam Shah wrote: > 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) { I had meant changing the map key to just host as port does not make a difference. But if all cases do not use hcaturi, then this is fine to avoid changing in too many places. - Rohini ----------------------------------------------------------- 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 > >
