You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Jonathan Maron <jm...@hortonworks.com> on 2015/07/09 20:06:06 UTC
Review Request 36359: Create configuration properties for HTTP and
HTTPS server ports
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/
-----------------------------------------------------------
Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
Bugs: ATLAS-36
https://issues.apache.org/jira/browse/ATLAS-36
Repository: atlas
Description
-------
Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
Diffs
-----
src/conf/application.properties 6c4c7d2
webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
Diff: https://reviews.apache.org/r/36359/diff/
Testing
-------
Installed a local server and observed the configuration properties working.
Added a unit test for port selection logic (MainIT).
Thanks,
Jonathan Maron
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Shwetha GS <ss...@hortonworks.com>.
> On July 13, 2015, 10:48 a.m., Shwetha GS wrote:
> > For hive hook, there is a config:
> > <property>
> > <name>atlas.rest.address</name>
> > <value>http://localhost:21000/</value>
> > </property>
> >
> >
> > Can we use the same config to check if its ssl or not, get the port and also bind to an address?
>
> Jonathan Maron wrote:
> We might be able to do that, but from a management interface it may not be ideal and would probably run contrary to the way in which users expect to manage the assignment of ports (see yarn and hdfs as examples). I think it is preferable to have persisted values for each transport type since that is a familiar and well-understood mechanism for configuration in a Hadoop cluster.
Even hdfs and yarn use the full endpoint configuration to bind the port
<property>
<description>The address of the applications manager interface in the RM.</description>
<name>yarn.resourcemanager.address</name>
<value>${yarn.resourcemanager.hostname}:8032</value>
</property>
<property>
<description>The hostname of the RM.</description>
<name>yarn.resourcemanager.hostname</name>
<value>0.0.0.0</value>
</property>
- Shwetha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91458
-----------------------------------------------------------
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Jonathan Maron <jm...@hortonworks.com>.
> On July 13, 2015, 10:48 a.m., Shwetha GS wrote:
> > For hive hook, there is a config:
> > <property>
> > <name>atlas.rest.address</name>
> > <value>http://localhost:21000/</value>
> > </property>
> >
> >
> > Can we use the same config to check if its ssl or not, get the port and also bind to an address?
>
> Jonathan Maron wrote:
> We might be able to do that, but from a management interface it may not be ideal and would probably run contrary to the way in which users expect to manage the assignment of ports (see yarn and hdfs as examples). I think it is preferable to have persisted values for each transport type since that is a familiar and well-understood mechanism for configuration in a Hadoop cluster.
>
> Shwetha GS wrote:
> Even hdfs and yarn use the full endpoint configuration to bind the port
> <property>
> <description>The address of the applications manager interface in the RM.</description>
> <name>yarn.resourcemanager.address</name>
> <value>${yarn.resourcemanager.hostname}:8032</value>
> </property>
> <property>
> <description>The hostname of the RM.</description>
> <name>yarn.resourcemanager.hostname</name>
> <value>0.0.0.0</value>
> </property>
I was talking specifically about the pattern of enabling SSL via a flag (which we do), and defining an associated port, much like HDFS, for example:
<property>
<name>dfs.http.policy</name>
<value>HTTPS_ONLY</value>
</property>
<property>
<name>dfs.https.port</name>
<value>50470</value>
</property>
Given the time differences and its effect on reviews (this simple change has now been in review for 6 days) I think that:
1) This is probably not a valid issue to prolong this review
2) If users complain we can certainly address a change of the approach with a subsequent JIRA.
I will therefore proceed with a commit.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91458
-----------------------------------------------------------
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Jonathan Maron <jm...@hortonworks.com>.
> On July 13, 2015, 10:48 a.m., Shwetha GS wrote:
> > For hive hook, there is a config:
> > <property>
> > <name>atlas.rest.address</name>
> > <value>http://localhost:21000/</value>
> > </property>
> >
> >
> > Can we use the same config to check if its ssl or not, get the port and also bind to an address?
We might be able to do that, but from a management interface it may not be ideal and would probably run contrary to the way in which users expect to manage the assignment of ports (see yarn and hdfs as examples). I think it is preferable to have persisted values for each transport type since that is a familiar and well-understood mechanism for configuration in a Hadoop cluster.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91458
-----------------------------------------------------------
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Shwetha GS <ss...@hortonworks.com>.
> On July 13, 2015, 10:48 a.m., Shwetha GS wrote:
> > For hive hook, there is a config:
> > <property>
> > <name>atlas.rest.address</name>
> > <value>http://localhost:21000/</value>
> > </property>
> >
> >
> > Can we use the same config to check if its ssl or not, get the port and also bind to an address?
>
> Jonathan Maron wrote:
> We might be able to do that, but from a management interface it may not be ideal and would probably run contrary to the way in which users expect to manage the assignment of ports (see yarn and hdfs as examples). I think it is preferable to have persisted values for each transport type since that is a familiar and well-understood mechanism for configuration in a Hadoop cluster.
>
> Shwetha GS wrote:
> Even hdfs and yarn use the full endpoint configuration to bind the port
> <property>
> <description>The address of the applications manager interface in the RM.</description>
> <name>yarn.resourcemanager.address</name>
> <value>${yarn.resourcemanager.hostname}:8032</value>
> </property>
> <property>
> <description>The hostname of the RM.</description>
> <name>yarn.resourcemanager.hostname</name>
> <value>0.0.0.0</value>
> </property>
>
> Jonathan Maron wrote:
> I was talking specifically about the pattern of enabling SSL via a flag (which we do), and defining an associated port, much like HDFS, for example:
>
> <property>
> <name>dfs.http.policy</name>
> <value>HTTPS_ONLY</value>
> </property>
>
> <property>
> <name>dfs.https.port</name>
> <value>50470</value>
> </property>
>
> Given the time differences and its effect on reviews (this simple change has now been in review for 6 days) I think that:
>
> 1) This is probably not a valid issue to prolong this review
> 2) If users complain we can certainly address a change of the approach with a subsequent JIRA.
>
> I will therefore proceed with a commit.
I guess the reason it has this config is because it hosts server on both http and https at the same time. For our case, we enabled either http or https, it makes sense to read from url. Will create another bug
- Shwetha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91458
-----------------------------------------------------------
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Shwetha GS <ss...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91458
-----------------------------------------------------------
For hive hook, there is a config:
<property>
<name>atlas.rest.address</name>
<value>http://localhost:21000/</value>
</property>
Can we use the same config to check if its ssl or not, get the port and also bind to an address?
- Shwetha GS
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Jonathan Maron <jm...@hortonworks.com>.
> On July 10, 2015, 5:05 p.m., Suma Shivaprasad wrote:
> > webapp/src/main/java/org/apache/atlas/Main.java, line 103
> > <https://reviews.apache.org/r/36359/diff/1/?file=1003745#file1003745line103>
> >
> > Could remove protected to make it package private. Dont get what a protected static method means.
I'm not sure about the qualifier either, but making it protected enabled a better testing approach and didn't seem to expose the internals in a detrimental way.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91320
-----------------------------------------------------------
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Jonathan Maron <jm...@hortonworks.com>.
> On July 10, 2015, 5:05 p.m., Suma Shivaprasad wrote:
> > webapp/src/main/java/org/apache/atlas/Main.java, line 103
> > <https://reviews.apache.org/r/36359/diff/1/?file=1003745#file1003745line103>
> >
> > Could remove protected to make it package private. Dont get what a protected static method means.
>
> Jonathan Maron wrote:
> I'm not sure about the qualifier either, but making it protected enabled a better testing approach and didn't seem to expose the internals in a detrimental way.
Actually, I imagine "protected" means that the method can be statically invoked from child classes or from within the same package.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91320
-----------------------------------------------------------
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Jonathan Maron <jm...@hortonworks.com>.
> On July 10, 2015, 5:05 p.m., Suma Shivaprasad wrote:
> > webapp/src/main/java/org/apache/atlas/Main.java, line 103
> > <https://reviews.apache.org/r/36359/diff/1/?file=1003745#file1003745line103>
> >
> > Could remove protected to make it package private. Dont get what a protected static method means.
>
> Jonathan Maron wrote:
> I'm not sure about the qualifier either, but making it protected enabled a better testing approach and didn't seem to expose the internals in a detrimental way.
>
> Jonathan Maron wrote:
> Actually, I imagine "protected" means that the method can be statically invoked from child classes or from within the same package.
>
> Suma Shivaprasad wrote:
> Since we need the method only in MainIT, the test which is in same package, we can drop the protected and make it
> static int getApplicationPort(CommandLine cmd,.... IMO. No need to have protected.
Fine.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91320
-----------------------------------------------------------
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Suma Shivaprasad <su...@gmail.com>.
> On July 10, 2015, 5:05 p.m., Suma Shivaprasad wrote:
> > webapp/src/main/java/org/apache/atlas/Main.java, line 103
> > <https://reviews.apache.org/r/36359/diff/1/?file=1003745#file1003745line103>
> >
> > Could remove protected to make it package private. Dont get what a protected static method means.
>
> Jonathan Maron wrote:
> I'm not sure about the qualifier either, but making it protected enabled a better testing approach and didn't seem to expose the internals in a detrimental way.
>
> Jonathan Maron wrote:
> Actually, I imagine "protected" means that the method can be statically invoked from child classes or from within the same package.
Since we need the method only in MainIT, the test which is in same package, we can drop the protected and make it
static int getApplicationPort(CommandLine cmd,.... IMO. No need to have protected.
- Suma
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91320
-----------------------------------------------------------
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>
Re: Review Request 36359: Create configuration properties for HTTP
and HTTPS server ports
Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36359/#review91320
-----------------------------------------------------------
webapp/src/main/java/org/apache/atlas/Main.java (line 103)
<https://reviews.apache.org/r/36359/#comment144634>
Could remove protected to make it package private. Dont get what a protected static method means.
- Suma Shivaprasad
On July 9, 2015, 6:06 p.m., Jonathan Maron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36359/
> -----------------------------------------------------------
>
> (Updated July 9, 2015, 6:06 p.m.)
>
>
> Review request for atlas, Arpit Gupta and Seetharam Venkatesh.
>
>
> Bugs: ATLAS-36
> https://issues.apache.org/jira/browse/ATLAS-36
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Distinct HTTP and HTTPS persisted port properties will make the configuration clearer and allow for a better experience from management interfaces such as Ambari.
>
>
> Diffs
> -----
>
> src/conf/application.properties 6c4c7d2
> webapp/src/main/java/org/apache/atlas/Main.java 1dd17b4
> webapp/src/test/java/org/apache/atlas/MainIT.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/36359/diff/
>
>
> Testing
> -------
>
> Installed a local server and observed the configuration properties working.
> Added a unit test for port selection logic (MainIT).
>
>
> Thanks,
>
> Jonathan Maron
>
>