> On Jan. 26, 2016, 9:39 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/RepositoryVersionEntity.java, > > lines 72-73 > > <https://reviews.apache.org/r/42824/diff/1/?file=1221847#file1221847line72> > > > > This implies that there will never be a repo created from an XML > > without an XSD. We good with that?
Yes. XSD are absolutely required since these files are generated outside Ambari. The parsing XML logic enforces a valid XSD file. Unfortunately I can't mark the column as not-null since we need backward compatibility within repo_version table. > On Jan. 26, 2016, 9:39 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java, > > line 268 > > <https://reviews.apache.org/r/42824/diff/1/?file=1221844#file1221844line268> > > > > Should this logic be encapsulated in the entity so that it's > > consistently set? I think that makes sense. Will fix. Ideally I'd like display name to be set in the XML, but couldn't convince myself of that requirement. > On Jan. 26, 2016, 9:39 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java, > > line 247 > > <https://reviews.apache.org/r/42824/diff/1/?file=1221844#file1221844line247> > > > > Heap concerns here? None. A "massive" file will still be less that 64k. This was covered in a different patch. > On Jan. 26, 2016, 9:39 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/VersionDefinitionService.java, > > line 50 > > <https://reviews.apache.org/r/42824/diff/1/?file=1221841#file1221841line50> > > > > GETs shouldn't need a `String body` Will fix. > On Jan. 26, 2016, 9:39 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/VersionDefinitionService.java, > > line 41 > > <https://reviews.apache.org/r/42824/diff/1/?file=1221841#file1221841line41> > > > > GETs shouldn't need a `String body` Will fix. > On Jan. 26, 2016, 9:39 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java, > > line 277 > > <https://reviews.apache.org/r/42824/diff/1/?file=1221844#file1221844line277> > > > > Doc. Will be in another jira, I forgot that was there. - Nate ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42824/#review116521 ----------------------------------------------------------- On Jan. 26, 2016, 4:37 p.m., Nate Cole wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42824/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2016, 4:37 p.m.) > > > Review request for Ambari, Alejandro Fernandez and Jonathan Hurley. > > > Bugs: AMBARI-14804 > https://issues.apache.org/jira/browse/AMBARI-14804 > > > Repository: ambari > > > Description > ------- > > For creating repositories from a Version Definition file, create a new > endpoint. This is because unlike repo management, the stack is not known in > advance. > > There are some validations missing from this code, but need to give the UI > folks a "good" API to start with. > > Still just a couple of reviewers until stabilization. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java > c7c5d61 > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/VersionDefinitionResourceDefinition.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/VersionDefinitionService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java > cce3764 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RepositoryVersionResourceProvider.java > 92b14b7 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/VersionDefinitionResourceProvider.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java > e367afe > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RepositoryVersionDAO.java > c37abb5 > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/RepositoryVersionEntity.java > e2e455b > > ambari-server/src/main/java/org/apache/ambari/server/state/repository/Release.java > 450fd95 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RepositoryVersionResourceProviderTest.java > 12d2091 > > Diff: https://reviews.apache.org/r/42824/diff/ > > > Testing > ------- > > Manual. Automated: > > Tests run: 3759, Failures: 0, Errors: 0, Skipped: 31 > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 31:58.624s > [INFO] Finished at: Tue Jan 26 16:11:54 EST 2016 > [INFO] Final Memory: 34M/1397M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Nate Cole > >
