> 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.
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). - Tim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51238/#review147153 ----------------------------------------------------------- On Aug. 26, 2016, 9:25 a.m., Balázs Bence Sári wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51238/ > ----------------------------------------------------------- > > (Updated Aug. 26, 2016, 9:25 a.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/api/services/AmbariMetaInfo.java > 92d47df > > ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelper.java > 36a2d99 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java > 63c99c6 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java > 097f01c > > ambari-server/src/main/java/org/apache/ambari/server/controller/RepositoryResponse.java > 30bd0db > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RepositoryResourceProvider.java > 3b5b0a7 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java > 02fc2ec > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/RepositoryEntity.java > 49d53a9 > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/RepositoryVersionEntity.java > 25aa62b > 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 > bfba021 > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java > 0606f2a > > 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/LatestRepoCallable.java > 3c7c001 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/RepositoryXml.java > 4a0ae3b > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/RepositoryVersionHelper.java > 50d6028 > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java > 3b5ed28 > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog220.java > d806dde > ambari-server/src/main/resources/version_definition.xsd bd49028 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RepositoryResourceProviderTest.java > 3a7b19b > > 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/java/org/apache/ambari/server/state/stack/upgrade/RepositoryVersionHelperTest.java > 8d82d09 > > 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/org/apache/ambari/server/stack/UpdateActiveRepoVersionOnStartupTest_initialRepos.xml > 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 > >
