> On Aug. 29, 2016, 5:27 p.m., Nate Cole wrote:
> > I was under the impression that applying an m-pack would DIRECTLY update 
> > repoinfo.xml on the filesystem.  Has the design changed since then?
> > 
> > What is the purpose of carrying service version around with the repo?  When 
> > updating m-packs, that just means that there's that much more work to do to 
> > get it right.
> > 
> > How are you going to prevent an m-pack from overwriting an existing repo? 
> > (What if an m-pack names a repo "HDP-UTILS" or something - then they just 
> > blew over ours)?
> > 
> > You need to fix the code formatting so it's consistent and a bit easier to 
> > read.
> > 
> > In general, I don't like the idea of using service name to tie a repo to a 
> > service.  Maybe name this "reference" or a different indicator of where 
> > this repo came from.
> 
> Jayush Luniya wrote:
>     RE: I was under the impression that applying an m-pack would DIRECTLY 
> update repoinfo.xml on the filesystem.
>     Personally, it would be better if we can avoid updates to the stack's 
> repoinfo.xml and have the support to define repos at service level.
> 
> Balázs Bence Sári wrote:
>     My implementation is based on this comment to the Jira (by Srimanth):
>     
>     "Discussed with Nate Cole and Jayush Luniya, and the approach for 
> custom-services to specify their own repo location will be to provide a 
> /repos/repoinfo.xml inside the stack-version they will be in. This repo file 
> will be loaded by Ambari during startup into the 
> /api/v1/stacks/HDP/versions/2.4/repository_versions repos. Service repo files 
> have a restriction that their (repo-name, base-url) locations should be 
> unique and not conflict. When conflicts do occur, they will not be loaded 
> into the stacks model.
>     Now the management-pack will provide such repos/ folder in 
> mpacks/custom-services/8.0.0/repos which will be linked into the stacks/ 
> folder.
>     
> ambari/ambari-server/src/main/resources/stacks/HDP/2.3/services/SERVICE_NAME/repos
>  -> mpacks/custom-services/8.0.0/repos"
>     
>     Overwriting a stack repo is prohibited in the implementation.
>     
>     service_name and service_version come from Srimanth, I can rename or 
> discard them. What should happen with them? Should I delete service_version 
> and rename service_name to reference? My two cents are that it's worth 
> keeping one to indicate that the repo does not come with the stack.
> 
> Nate Cole wrote:
>     Association should be with the service itself (I think that's what Jayush 
> is saying), and we can access those as part of the stack endpoint for the 
> service, but should not be integrated with the stack definition.  We should 
> then include that repo info when we create the tasks to agents.
> 
> Balázs Bence Sári wrote:
>     I think that's a bit different than how it was in the Jira comment (at 
> least to my interpetation) which I looked like a result of consensus. I can 
> implement it however. 
>     
>     Also, we also need to make sure the user can overwrite those repo url's 
> when installing a cluster (Microsoft wants no public repo, repo locatinon in 
> the final mpack will be empty, corporates are expected to create their 
> private repos). With the current implementation, 
> api/v1/stacks/HDP/versions/2.3/operating_systems/redhat6 will return the 
> service repo along the stack repos and the user can override it on the UI. If 
> I move the repo to the service, should this API call still return it? 
> (guessing that UI uses this API to get the repos).
> 
> Nate Cole wrote:
>     We can aggregate them in the API response, but I don't know that they 
> should be stored with the stack repos.
> 
> Tim Thorpe wrote:
>     I added stack extensions 
> (https://cwiki.apache.org/confluence/display/AMBARI/Extensions) for the 
> purpose of allowing shared resources for a collection of custom services.  
> The way you are implementing service level repositories seems wrong.  If you 
> have multiple services that require a custom repository, then you are 
> randomly selecting which service's repoinfo.xml you will use.  If it really 
> is a shared repository, it should be included at the extension level.
>     
>     In general, I think we should be encouraging third parties to deliver 
> their custom services in a consistent way (extensions installed with mpacks).
> 
> Jayush Luniya wrote:
>     @Tim, 
>     The inherent assumption here is that multiple custom services from a 
> vendor will be bundled in the same extension, which most likely wouldn't be 
> the case as it is quite likely that 3rd party vendors release different 
> custom services at different point in time. So if 2 custom services with bits 
> in same repo are released in 2 different extensions, we would land up in the 
> same situation. We need to support repo at a service definition level, and 
> support the different cases accordingly.

@Jayush, I know some vendors ship custom services as released package products. 
 If they choose to package them separately then it is just as easy to change 
the repo URL.  

I don't think it is a good idea to store the service name and version with the 
repository unless you allow duplication of repo URLs for services.  By this I 
mean if there is a new repo version table for the stack or services introduced, 
it could include repo_version_id, stack_name, stack_version, service_name, 
repo_id, repo_name and url.  There would be 3 unique constraints (stack_name, 
stack_version, service_name, repo_id), (stack_name, stack_version, 
service_name, repo_name) and (stack_name, stack_version, service_name, url).

Ideally the whole repo database code would be simplified.  I don't really 
understand the point of storing multiple json files in the DB.  Once the table 
is populated, all that really should be needed is a list of URLs to use for a 
particular stack version.  So we could use one table to store all the repos 
like the one I mentioned above or we could drop the service_name column and 
filter the unique URLs at the java level prior to saving them to the DB.

Because we now support patch management, this needs to be taken into account.  
Services should be able to supply a latest URL.

Lastly, this needs to be tested with extension services.


- Tim


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


On Sept. 5, 2016, 1:26 p.m., Balázs Bence Sári wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51238/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2016, 1:26 p.m.)
> 
> 
> Review request for Ambari, Jayush Luniya, Nate Cole, Sumit Mohanty, and 
> Sebastian Toader.
> 
> 
> Bugs: AMBARI-15538
>     https://issues.apache.org/jira/browse/AMBARI-15538
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Implementation contains the following things:
> - On startup, add-on service repos are loaded into the stack model
> - On startup, add-on service repos are merged into the VDF's downloaded by 
> LatestRepoCallable
> - On startup, if the is an existing cluster, it's repository version entity 
> is potentially updated with new add-on service repos.
> - Repository definitions contain two new optional fields: service_name and 
> service_version. (null for stack repositories) 
> - Small changes on the Mictrosoft-R mpack (supoorts HDP-2.4 and 2.5)
> - New unit tests
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelper.java
>  9f7419c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  b1fd592 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
>  d20b1d7 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java
>  02fc2ec 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/RepoUtil.java 
> PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  3acc617 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 23b0218 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  7bcd08b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/UpdateActiveRepoVersionOnStartup.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/RepositoryInfo.java
>  29776ed 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 
> 14ff9de 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/RepositoryVersionHelper.java
>  50d6028 
>   ambari-server/src/main/resources/version_definition.xsd bd49028 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/RepoUtilTest.java 
> PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerCommonServicesTest.java
>  1d73ff3 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackModuleTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/UpdateActiveRepoVersionOnStartupTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/common-services/ADDON/1.0/configuration/addon-env.xml
>  PRE-CREATION 
>   ambari-server/src/test/resources/common-services/ADDON/1.0/metainfo.xml 
> PRE-CREATION 
>   
> ambari-server/src/test/resources/org/apache/ambari/server/stack/UpdateActiveRepoVersionOnStartupTest_initialRepos.json
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_common_services/HDP/0.2/services/ADDON/metainfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_common_services/HDP/0.2/services/ADDON/repos/repoinfo.xml
>  PRE-CREATION 
>   
> contrib/management-packs/microsoft-r_mpack/src/main/resources/common-services/MICROSOFT_R/8.0.0/configuration/microsoft-r-env.xml
>  PRE-CREATION 
>   
> contrib/management-packs/microsoft-r_mpack/src/main/resources/common-services/MICROSOFT_R/8.0.0/package/scripts/microsoft_r.py
>  61ea96b 
>   
> contrib/management-packs/microsoft-r_mpack/src/main/resources/custom-services/MICROSOFT_R/8.0.0/repos/repoinfo.xml
>  PRE-CREATION 
>   contrib/management-packs/microsoft-r_mpack/src/main/resources/mpack.json 
> f90ccce 
> 
> Diff: https://reviews.apache.org/r/51238/diff/
> 
> 
> Testing
> -------
> 
> - Manually tested
> - Wrote new unit tests
> - All unit tests passed except two which were failing in CI builds as well.
> 
> 
> Thanks,
> 
> Balázs Bence Sári
> 
>

Reply via email to