Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Madhan Neethiraj]

> On May 16, 2018, 2:55 p.m., Madhan Neethiraj wrote:
> > distro/pom.xml
> > Lines 123 (patched)
> > 
> >
> > This results in omag-server binaries to be includes in Atlas packaging. 
> > Unless the implementation is hardened with sufficient testing, I would 
> > strongly recommend to not package this module.
> 
> Nigel Jones wrote:
> This change has already been committed into master & since it was prior 
> to the branch, into 1.0.
> 
> The binary (and dependencies) are indeed included in a distinct archive 
> along with the other archives as part of the distribution. The OMAG 
> application however runs independently of atlas. It's a standalone process 
> that cannot access Atlas data, as the atlas local connector has not yet been 
> checked in (nor that of any other repository). It runs on a separate port. It 
> cannot interact with hbase,  It facilitates development of repository 
> connectors. As such we felt the risk was very low.
> 
> However if we want to remove it for 1.0 we can, we could either directly 
> reverse the patch, or I can build a new one for 1.0 with some or all of the 
> changes undone. I presume we would leave in master. Happy for you to raise a 
> JIRA or just reply here & I can arrange for that.

Nigel - this change can continue to remain in master branch. Can you please 
create a patch to revert the packaging change (distro/pom.xml) in branch-1.0? 
Thanks!


- Madhan


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


On May 13, 2018, 8:54 a.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 13, 2018, 8:54 a.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/6/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

> On May 16, 2018, 3:55 p.m., Madhan Neethiraj wrote:
> > distro/pom.xml
> > Lines 123 (patched)
> > 
> >
> > This results in omag-server binaries to be includes in Atlas packaging. 
> > Unless the implementation is hardened with sufficient testing, I would 
> > strongly recommend to not package this module.

This change has already been committed into master & since it was prior to the 
branch, into 1.0.

The binary (and dependencies) are indeed included in a distinct archive along 
with the other archives as part of the distribution. The OMAG application 
however runs independently of atlas. It's a standalone process that cannot 
access Atlas data, as the atlas local connector has not yet been checked in 
(nor that of any other repository). It runs on a separate port. It cannot 
interact with hbase,  It facilitates development of repository connectors. As 
such we felt the risk was very low.

However if we want to remove it for 1.0 we can, we could either directly 
reverse the patch, or I can build a new one for 1.0 with some or all of the 
changes undone. I presume we would leave in master. Happy for you to raise a 
JIRA or just reply here & I can arrange for that.


- Nigel


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


On May 13, 2018, 9:54 a.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 13, 2018, 9:54 a.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/6/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Madhan Neethiraj]

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




distro/pom.xml
Lines 123 (patched)


This results in omag-server binaries to be includes in Atlas packaging. 
Unless the implementation is hardened with sufficient testing, I would strongly 
recommend to not package this module.


- Madhan Neethiraj


On May 13, 2018, 8:54 a.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 13, 2018, 8:54 a.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/6/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

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

(Updated May 13, 2018, 9:54 a.m.)


Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
Mandy Chessell.


Changes
---

Updated patch to match jira (only a minor whitespace change from editor)


Repository: atlas


Description
---

Added OMAG Server to distribution with an easy to launch jar
(See JIRA for more information)


Diffs (updated)
-

  distro/pom.xml 6431fd86d 
  distro/src/main/assemblies/omag-server.xml PRE-CREATION 
  omag-server/README.md PRE-CREATION 
  omag-server/pom.xml 4c1c98aa3 


Diff: https://reviews.apache.org/r/67060/diff/6/

Changes: https://reviews.apache.org/r/67060/diff/5-6/


Testing
---

Built atlas
Checked correct files in distribution archive
Checked OMAG server launches with default config


Thanks,

Nigel Jones

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

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

(Updated May 11, 2018, 12:58 p.m.)


Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
Mandy Chessell.


Changes
---

removed *.patch files erroneously added when recreating to remove whitespace. 
SHould be good now. Tested build, launch. All clean.


Repository: atlas


Description
---

Added OMAG Server to distribution with an easy to launch jar
(See JIRA for more information)


Diffs (updated)
-

  distro/pom.xml 6431fd86d 
  distro/src/main/assemblies/omag-server.xml PRE-CREATION 
  omag-server/README.md PRE-CREATION 
  omag-server/pom.xml 4c1c98aa3 


Diff: https://reviews.apache.org/r/67060/diff/5/

Changes: https://reviews.apache.org/r/67060/diff/4-5/


Testing
---

Built atlas
Checked correct files in distribution archive
Checked OMAG server launches with default config


Thanks,

Nigel Jones

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

> On May 11, 2018, 12:30 p.m., Nigel Jones wrote:
> > ATLAS-2668a.patch
> > Lines 1 (patched)
> > 
> >
> > !! Patch is still in the diff. inadvertently must have added when 
> > trying to fix whitespace. Original patch was otherwise good.
> > 
> > Now will update to remove file from patch
> > 
> > Hold off from test/ship for now

resolved


- Nigel


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


On May 11, 2018, 12:58 p.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 11, 2018, 12:58 p.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/5/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

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




ATLAS-2668a.patch
Lines 1 (patched)


!! Patch is still in the diff. inadvertently must have added when trying to 
fix whitespace. Original patch was otherwise good.

Now will update to remove file from patch

Hold off from test/ship for now


- Nigel Jones


On May 11, 2018, 12:26 p.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 11, 2018, 12:26 p.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   ATLAS-2668a.patch PRE-CREATION 
>   ATLAS-2668b.patch PRE-CREATION 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/4/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

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

(Updated May 11, 2018, 12:26 p.m.)


Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
Mandy Chessell.


Changes
---

updated without patch file


Repository: atlas


Description
---

Added OMAG Server to distribution with an easy to launch jar
(See JIRA for more information)


Diffs (updated)
-

  ATLAS-2668a.patch PRE-CREATION 
  ATLAS-2668b.patch PRE-CREATION 
  distro/pom.xml 6431fd86d 
  distro/src/main/assemblies/omag-server.xml PRE-CREATION 
  omag-server/README.md PRE-CREATION 
  omag-server/pom.xml 4c1c98aa3 


Diff: https://reviews.apache.org/r/67060/diff/4/

Changes: https://reviews.apache.org/r/67060/diff/3-4/


Testing
---

Built atlas
Checked correct files in distribution archive
Checked OMAG server launches with default config


Thanks,

Nigel Jones

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

> On May 11, 2018, 10:39 a.m., David Radley wrote:
> > omag-server/README.md
> > Lines 25 (patched)
> > 
> >
> > If you lose this whitespace - then we will not get thew white space 
> > warning on patch apply
> 
> Nigel Jones wrote:
> Odd. hadn't noticed that. I still need to understand how it gets there. 
> Will do.

Difficult to eliminate completely especially in README.md as intellij likes 
adding space indentation to match prior line even at end of file - in my case 
indentation was to get block quoting.

Trying to fix this has led to other errors so I propose we leave this as-is for 
now (at least as per current patch). Perhaps there are some intellij settings 
we can tweak to reduce the likelihood of whitespace


- Nigel


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


On May 11, 2018, 12:26 p.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 11, 2018, 12:26 p.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   ATLAS-2668a.patch PRE-CREATION 
>   ATLAS-2668b.patch PRE-CREATION 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/4/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

> On May 11, 2018, 12:20 p.m., David Radley wrote:
> > ATLAS-2668a.patch
> > Lines 1 (patched)
> > 
> >
> > looks like a patch has got into the diff

Yes. My bad. I forgot to delete the patch when making the followon diff (it's 
not in the local git commit, so git-wise I look ok). Realised before the 
comment but you were too quick :-)  Removed now. It's quite easy to make little 
mistakes like this with the tool but practice should help ...


- Nigel


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


On May 11, 2018, 12:18 p.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 11, 2018, 12:18 p.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   ATLAS-2668a.patch PRE-CREATION 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/3/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[David Radley]

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




ATLAS-2668a.patch
Lines 1 (patched)


looks like a patch has got into the diff


- David Radley


On May 11, 2018, 11:18 a.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 11, 2018, 11:18 a.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   ATLAS-2668a.patch PRE-CREATION 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/3/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[David Radley]

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


Ship it!




Please make the changes then ship it

- David Radley


On May 11, 2018, 11:18 a.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 11, 2018, 11:18 a.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   ATLAS-2668a.patch PRE-CREATION 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/3/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

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

(Updated May 11, 2018, 12:18 p.m.)


Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
Mandy Chessell.


Changes
---

updated again as I noticed rogue whitespace in readme


Repository: atlas


Description
---

Added OMAG Server to distribution with an easy to launch jar
(See JIRA for more information)


Diffs (updated)
-

  ATLAS-2668a.patch PRE-CREATION 
  distro/pom.xml 6431fd86d 
  distro/src/main/assemblies/omag-server.xml PRE-CREATION 
  omag-server/README.md PRE-CREATION 
  omag-server/pom.xml 4c1c98aa3 


Diff: https://reviews.apache.org/r/67060/diff/3/

Changes: https://reviews.apache.org/r/67060/diff/2-3/


Testing
---

Built atlas
Checked correct files in distribution archive
Checked OMAG server launches with default config


Thanks,

Nigel Jones

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

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

(Updated May 11, 2018, 12:15 p.m.)


Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
Mandy Chessell.


Changes
---

Updated as per comments


Repository: atlas


Description
---

Added OMAG Server to distribution with an easy to launch jar
(See JIRA for more information)


Diffs (updated)
-

  distro/pom.xml 6431fd86d 
  distro/src/main/assemblies/omag-server.xml PRE-CREATION 
  omag-server/README.md PRE-CREATION 
  omag-server/pom.xml 4c1c98aa3 


Diff: https://reviews.apache.org/r/67060/diff/2/

Changes: https://reviews.apache.org/r/67060/diff/1-2/


Testing
---

Built atlas
Checked correct files in distribution archive
Checked OMAG server launches with default config


Thanks,

Nigel Jones

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[David Radley]

Hi Nigel,
Thanks for the explanations. I have indicated ship it! Once you make your 
amendments - I intend to commit this tomorrow if there are no further 
issues raised today, 

fyi - I notice you copied the dev list with incubator in the name - I have 
copied the non-incubator dev list. 
all the best, David. 


From:   Nigel Jones <nigel.l.jo...@gmail.com>
To: Mandy Chessell <mandy_chess...@uk.ibm.com>, Graham Wallis 
<graham_wal...@uk.ibm.com>, Madhan Neethiraj <mad...@apache.org>, David 
Radley <david...@apache.org>
Cc: atlas <d...@atlas.incubator.apache.org>
Date:   11/05/2018 10:51
Subject:    Re: Review Request 67060: ATLAS-2668: Add OMAG Server to 
distribution
Sent by:Nigel Jones <nore...@reviews.apache.org>



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

On May 11th, 2018, 10:39 a.m. BST, David Radley wrote:

distro/src/main/assemblies/omag-server.xml (Diff revision 1) 


29

I am wondering if you should add   and 
 like KafkaHook does. We should ensure that the jar and sh 
files are executable and the readme is not.


I see
-rw-r--r--  1 david  staff  1080 11 May 09:48 README.md
-rw-r--r--  1 david  staff  8022 11 May 09:55 
omag-server-1.0.0-SNAPSHOT-sources.jar
-rw-r--r--  1 david  staff  6561 11 May 09:55 
omag-server-1.0.0-SNAPSHOT-test-sources.jar
-rw-r--r--  1 david  staff  17222951 11 May 09:55 
omag-server-1.0.0-SNAPSHOT.jar


It doesn to look like the jar is executable.
Also why have we got the sources and test sources jar files?
On May 11th, 2018, 10:41 a.m. BST, David Radley wrote:
As it works - I guess we do not need to make it executable. Please could 
you review deleting the otehr jars
Intriguingly when I build it I do not see those jars in the distribution 
which only contains the files as per the jira ie
? 
~/src/atlas/distro/target/apache-atlas-1.0.0-SNAPSHOT-omag-server/omag-server-1.0.0-SNAPSHOT
 
[master ?·3?·1|…1? 3]
10:49 $ ls
README.md  omag-server-1.0.0-SNAPSHOT.jar


I did check this beforehand.. I'll do a clean build again to double check


Note I am referring here to what goes in the distribution (under 
distro/target)... I think I'd leave what's in the component build 
(omag-server/target)

On May 11th, 2018, 10:39 a.m. BST, David Radley wrote:

distro/src/main/assemblies/omag-server.xml (Diff revision 1) 


38
../omag-server
I see none of the other assemblies use ../ I am not sure why we need to?
This is the link to the location of the artifacts we want to pull into the 
distibution . Others do use it such as atlas-storm-hook-bridge.xml, so 
does hbase. Ok?

On May 11th, 2018, 10:39 a.m. BST, David Radley wrote:

omag-server/README.md (Diff revision 1) 


22
**Launching the standalone server**
I suggest changing this to be launching the standalone Open Metadata And 
Governance (OMAG) Server.
Fair comment, though as there's no regression here could do in another 
jira for expediency? Can remake patch once we are happy with other issues?

On May 11th, 2018, 10:39 a.m. BST, David Radley wrote:

omag-server/README.md (Diff revision 1) 


25
 
If you lose this whitespace - then we will not get thew white space 
warning on patch apply
Odd. hadn't noticed that. I still need to understand how it gets there. 
Will do.

On May 11th, 2018, 10:39 a.m. BST, David Radley wrote:

omag-server/pom.xml (Diff revision 1) 
124
org.apache.maven.plugins
124
org.springframework.boot
I do not understand this change. How does this effect the Atlas build?
It's to do with making an executable jar. Previously the maven-jar-plugin 
was being used, but would not build a jar you could run - nor did it 
include dependencies. To do that you can normally use the 
maven-shade-plugin instead (as we see elsewhere in atlas, and as I used 
for the gaian ranger plugin). Here you can built a composite jar with 
dependencies, and can also set the main class in the manifest. However in 
a spring environment this fails as additional configuration data is 
needed. This is where the spring maven plugin comes to the rescue and 
builds a correct jar so you can just now run the omag server as per the 
readme. The JIRA has a little more explanation

- Nigel

On May 10th, 2018, 5:23 p.m. BST, Nigel Jones wrote:

Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, 
and Mandy Chessell.
By Nigel Jones.
Updated May 10, 2018, 5:23 p.m.
Repository: atlas 
Description 

Added OMAG Server to distribution with an easy to launch jar
(See JIRA for more information)
Testing 

Built atlas
Checked correct files in distribution archive
Checked OMAG server launches with default config
Diffs 
distro/pom.xml (6431fd86d)
distro/src/main/assemblies/omag-server.xml (PRE-CREATION)
omag-server/README.md (PRE-CREATION)
omag-server/pom.xml (4c1c98aa3)
View Diff



Unless stated otherwise above:
IBM Unit

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

> On May 11, 2018, 10:39 a.m., David Radley wrote:
> > omag-server/README.md
> > Lines 22 (patched)
> > 
> >
> > I suggest changing this to be launching the standalone Open Metadata 
> > And Governance (OMAG) Server.
> 
> Nigel Jones wrote:
> Fair comment, though as there's no regression here could do in another 
> jira for expediency? Can remake patch once we are happy with other issues?

On reflection I propose leaving as is for the following reasons
 * The title of the readme already uses the full name 'Open Metadata and 
Governance Server'
 * OMAG is already referred to in the readme as the abbreviation
 * The launching ... is only a sub heading within the tiny document
 * This is a placeholder for a more complete readme we should add about how to 
use the omag server - so consider it a bootstrap to get started (and an 
improvement on not having anything)
 * We should ultimately likely have full start/stop scripts, which should use 
the wording you describe. However we need to talk a little more on a jira/lists 
about what that might entail, so I decided to omit for expediency - and I think 
what we have here offers value now


- Nigel


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


On May 10, 2018, 5:23 p.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 10, 2018, 5:23 p.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/1/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

> On May 11, 2018, 10:39 a.m., David Radley wrote:
> > distro/src/main/assemblies/omag-server.xml
> > Lines 29 (patched)
> > 
> >
> > I am wondering if you should add   and 
> >  like KafkaHook does. We should ensure that the jar and 
> > sh files are executable and the readme is not.
> > 
> > I see
> > -rw-r--r--  1 david  staff  1080 11 May 09:48 README.md
> > -rw-r--r--  1 david  staff  8022 11 May 09:55 
> > omag-server-1.0.0-SNAPSHOT-sources.jar
> > -rw-r--r--  1 david  staff  6561 11 May 09:55 
> > omag-server-1.0.0-SNAPSHOT-test-sources.jar
> > -rw-r--r--  1 david  staff  17222951 11 May 09:55 
> > omag-server-1.0.0-SNAPSHOT.jar
> > 
> > 
> > It doesn to look like the jar is executable.
> > Also why have we got the sources and test sources jar files?
> 
> David Radley wrote:
> As it works - I guess we do not need to make it executable. Please could 
> you review deleting the otehr jars
> 
> Nigel Jones wrote:
> Intriguingly when I build it I do not see those jars in the distribution 
> which only contains the files as per the jira ie
> ? 
> ~/src/atlas/distro/target/apache-atlas-1.0.0-SNAPSHOT-omag-server/omag-server-1.0.0-SNAPSHOT
>  [master ?·3?·1|…1? 3]
> 10:49 $ ls
> README.md  omag-server-1.0.0-SNAPSHOT.jar
> 
> I did check this beforehand.. I'll do a clean build again to double check
> 
> Note I am referring here to what goes in the distribution (under 
> distro/target)... I think I'd leave what's in the component build 
> (omag-server/target)

On the permissions, I can add those, as it makes sense, but the jar was tested 
and does work


- Nigel


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


On May 10, 2018, 5:23 p.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 10, 2018, 5:23 p.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/1/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

> On May 11, 2018, 10:39 a.m., David Radley wrote:
> > distro/src/main/assemblies/omag-server.xml
> > Lines 29 (patched)
> > 
> >
> > I am wondering if you should add   and 
> >  like KafkaHook does. We should ensure that the jar and 
> > sh files are executable and the readme is not.
> > 
> > I see
> > -rw-r--r--  1 david  staff  1080 11 May 09:48 README.md
> > -rw-r--r--  1 david  staff  8022 11 May 09:55 
> > omag-server-1.0.0-SNAPSHOT-sources.jar
> > -rw-r--r--  1 david  staff  6561 11 May 09:55 
> > omag-server-1.0.0-SNAPSHOT-test-sources.jar
> > -rw-r--r--  1 david  staff  17222951 11 May 09:55 
> > omag-server-1.0.0-SNAPSHOT.jar
> > 
> > 
> > It doesn to look like the jar is executable.
> > Also why have we got the sources and test sources jar files?
> 
> David Radley wrote:
> As it works - I guess we do not need to make it executable. Please could 
> you review deleting the otehr jars

Intriguingly when I build it I do not see those jars in the distribution which 
only contains the files as per the jira ie
? 
~/src/atlas/distro/target/apache-atlas-1.0.0-SNAPSHOT-omag-server/omag-server-1.0.0-SNAPSHOT
 [master ?·3?·1|…1? 3]
10:49 $ ls
README.md  omag-server-1.0.0-SNAPSHOT.jar

I did check this beforehand.. I'll do a clean build again to double check

Note I am referring here to what goes in the distribution (under 
distro/target)... I think I'd leave what's in the component build 
(omag-server/target)


> On May 11, 2018, 10:39 a.m., David Radley wrote:
> > distro/src/main/assemblies/omag-server.xml
> > Lines 38 (patched)
> > 
> >
> > I see none of the other assemblies use ../ I am not sure why we need to?

This is the link to the location of the artifacts we want to pull into the 
distibution . Others do use it such as atlas-storm-hook-bridge.xml, so does 
hbase. Ok?


> On May 11, 2018, 10:39 a.m., David Radley wrote:
> > omag-server/README.md
> > Lines 22 (patched)
> > 
> >
> > I suggest changing this to be launching the standalone Open Metadata 
> > And Governance (OMAG) Server.

Fair comment, though as there's no regression here could do in another jira for 
expediency? Can remake patch once we are happy with other issues?


> On May 11, 2018, 10:39 a.m., David Radley wrote:
> > omag-server/README.md
> > Lines 25 (patched)
> > 
> >
> > If you lose this whitespace - then we will not get thew white space 
> > warning on patch apply

Odd. hadn't noticed that. I still need to understand how it gets there. Will do.


> On May 11, 2018, 10:39 a.m., David Radley wrote:
> > omag-server/pom.xml
> > Line 124 (original), 124 (patched)
> > 
> >
> > I do not understand this change. How does this effect the Atlas build?

It's to do with making an executable jar. Previously the maven-jar-plugin was 
being used, but would not build a jar you could run - nor did it include 
dependencies. To do that you can normally use the maven-shade-plugin instead 
(as we see elsewhere in atlas, and as I used for the gaian ranger plugin). Here 
you can built a composite jar with dependencies, and can also set the main 
class in the manifest. However in a spring environment this fails as additional 
configuration data is needed. This is where the spring maven plugin comes to 
the rescue and builds a correct jar so you can just now run the omag server as 
per the readme. The JIRA has a little more explanation


- Nigel


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


On May 10, 2018, 5:23 p.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 10, 2018, 5:23 p.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/1/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG 

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[David Radley]

> On May 11, 2018, 9:39 a.m., David Radley wrote:
> > distro/src/main/assemblies/omag-server.xml
> > Lines 29 (patched)
> > 
> >
> > I am wondering if you should add   and 
> >  like KafkaHook does. We should ensure that the jar and 
> > sh files are executable and the readme is not.
> > 
> > I see
> > -rw-r--r--  1 david  staff  1080 11 May 09:48 README.md
> > -rw-r--r--  1 david  staff  8022 11 May 09:55 
> > omag-server-1.0.0-SNAPSHOT-sources.jar
> > -rw-r--r--  1 david  staff  6561 11 May 09:55 
> > omag-server-1.0.0-SNAPSHOT-test-sources.jar
> > -rw-r--r--  1 david  staff  17222951 11 May 09:55 
> > omag-server-1.0.0-SNAPSHOT.jar
> > 
> > 
> > It doesn to look like the jar is executable.
> > Also why have we got the sources and test sources jar files?

As it works - I guess we do not need to make it executable. Please could you 
review deleting the otehr jars


- David


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


On May 10, 2018, 4:23 p.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 10, 2018, 4:23 p.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/1/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Re: Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[David Radley]

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




distro/src/main/assemblies/omag-server.xml
Lines 29 (patched)


I am wondering if you should add   and 
 like KafkaHook does. We should ensure that the jar and sh 
files are executable and the readme is not.

I see
-rw-r--r--  1 david  staff  1080 11 May 09:48 README.md
-rw-r--r--  1 david  staff  8022 11 May 09:55 
omag-server-1.0.0-SNAPSHOT-sources.jar
-rw-r--r--  1 david  staff  6561 11 May 09:55 
omag-server-1.0.0-SNAPSHOT-test-sources.jar
-rw-r--r--  1 david  staff  17222951 11 May 09:55 
omag-server-1.0.0-SNAPSHOT.jar

It doesn to look like the jar is executable.
Also why have we got the sources and test sources jar files?



distro/src/main/assemblies/omag-server.xml
Lines 38 (patched)


I see none of the other assemblies use ../ I am not sure why we need to?



omag-server/README.md
Lines 22 (patched)


I suggest changing this to be launching the standalone Open Metadata And 
Governance (OMAG) Server.



omag-server/README.md
Lines 25 (patched)


If you lose this whitespace - then we will not get thew white space warning 
on patch apply



omag-server/pom.xml
Line 124 (original), 124 (patched)


I do not understand this change. How does this effect the Atlas build?


- David Radley


On May 10, 2018, 4:23 p.m., Nigel Jones wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67060/
> ---
> 
> (Updated May 10, 2018, 4:23 p.m.)
> 
> 
> Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
> Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Added OMAG Server to distribution with an easy to launch jar
> (See JIRA for more information)
> 
> 
> Diffs
> -
> 
>   distro/pom.xml 6431fd86d 
>   distro/src/main/assemblies/omag-server.xml PRE-CREATION 
>   omag-server/README.md PRE-CREATION 
>   omag-server/pom.xml 4c1c98aa3 
> 
> 
> Diff: https://reviews.apache.org/r/67060/diff/1/
> 
> 
> Testing
> ---
> 
> Built atlas
> Checked correct files in distribution archive
> Checked OMAG server launches with default config
> 
> 
> Thanks,
> 
> Nigel Jones
> 
>

Review Request 67060: ATLAS-2668: Add OMAG Server to distribution

[Nigel Jones]

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

Review request for atlas, David Radley, Graham Wallis, Madhan Neethiraj, and 
Mandy Chessell.


Repository: atlas


Description
---

Added OMAG Server to distribution with an easy to launch jar
(See JIRA for more information)


Diffs
-

  distro/pom.xml 6431fd86d 
  distro/src/main/assemblies/omag-server.xml PRE-CREATION 
  omag-server/README.md PRE-CREATION 
  omag-server/pom.xml 4c1c98aa3 


Diff: https://reviews.apache.org/r/67060/diff/1/


Testing
---

Built atlas
Checked correct files in distribution archive
Checked OMAG server launches with default config


Thanks,

Nigel Jones