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 <fileSet> I am wondering if you should add <fileMode> and <directoryMode> 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 <directory>../omag-server</directory> 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 <groupId>org.apache.maven.plugins</groupId> 124 <groupId>org.springframework.boot</groupId> 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 United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU