Federico Simoncelli has posted comments on this change.
Change subject: core: add repo_file_id to repo_file_meta_data
......................................................................
Patch Set 2: (9 inline comments)
....................................................
File backend/manager/dbscripts/create_views.sql
Line 90: storage_domain_static.id as storage_domain_id,
Line 91: storage_domain_static.storage_domain_type as
storage_domain_type,
Line 92: storage_pool_iso_map.storage_pool_id as storage_pool_id,
Line 93: storage_pool_iso_map.status as storage_domain_status,
Line 94: repo_file_meta_data.repo_image_id as repo_image_id,
it is currently unused; repo_image_id (ex repo_file_name) is everything we need
in this view. If you're brave and you backtrack the need of this view you'll
discover that is (looks) redundant and it's used (at the far end) only by
fetchIsoDomains.
Line 95: repo_file_meta_data.size as size,
Line 96: repo_file_meta_data.date_created as date_created,
Line 97: repo_file_meta_data.last_refreshed as last_refreshed,
Line 98: repo_file_meta_data.file_type as file_type,
....................................................
File backend/manager/dbscripts/upgrade/03_03_0040_repo_file_id.sql
Line 1: -- The current file names from now on will be used as images id in the
ISO domains
Line 2: ALTER TABLE repo_file_meta_data RENAME COLUMN repo_file_name TO
repo_image_id;
Done
Line 1: -- The current file names from now on will be used as images id in the
ISO domains
Line 2: ALTER TABLE repo_file_meta_data RENAME COLUMN repo_file_name TO
repo_image_id;
Line 3: ALTER TABLE repo_file_meta_data ADD COLUMN repo_image_name VARCHAR(256);
Done
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/IsoDomainListSyncronizer.java
Line 515: repo_md.setSize(0);
Line 516:
repo_md.setRepoDomainId(repoStorageDomainId);
Line 517: repo_md.setDateCreated(null);
Line 518: repo_md.setRepoImageId(isoFile);
Line 519: repo_md.setRepoImageName(null);
I thought a little about this. Basically it should work like this:
ImageName is null: there's no friendly name for the image, use the ImageId to
list/display the image.
ImageName is not null: there's a friendly name specified for the image but
since it can be duplicated (it's not guaranteed to be unique), then we should
also give an hint about which image it is (at the moment that's done reporting
the initial part of ImageId, just as git does with short hashes).
Now considering this last case if we set ImageName = ImageId, then the
displayed name would be something like "myisoimagename.iso (myisoim)".
Please let me know if you have a better idea on how to handle this all.
Anyway (beside the image name displaying) I think that the safest way to
address this now is not to set an ImageName since in fact there's not an
additional friendly name to set.
Line 520: repo_md.setFileType(imageType);
Line 521:
repoFileMetaDataDao.addRepoFileMap(repo_md);
Line 522: }
Line 523: return true;
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
Line 606: ImageFileType.ISO);
Line 607: Version bestClusterVer = null;
Line 608: int bestToolVer = 0;
Line 609: for (RepoFileMetaData map : repoFilesMap) {
Line 610: String fileName = map.getRepoImageId() != null ?
map.getRepoImageId() : "";
Done
Line 611: Matcher matchToolPattern =
Line 612:
Pattern.compile(IsoDomainListSyncronizer.getRegexToolPattern()).matcher(fileName);
Line 613: if (matchToolPattern.find()) {
Line 614: // Get cluster version and tool version of Iso
tool.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/RepoFileMetaData.java
Line 140: if (repoImageName != null) {
Line 141: /* To provide an hint about the image id and at the same
time
Line 142: * maintain the image title short we just report 7
characters
Line 143: * of the id (similarly to what git does with hashes).
Line 144: */
Doing something different from the rest of the file will hurt my eyes :-)
Anyway no problem, I'm fixing this.
Line 145: return repoImageName + " (" + repoImageId.substring(0, 7)
+ ")";
Line 146: } else {
Line 147: return repoImageId;
Line 148: }
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/RepoFileMetaDataDAODbFacadeImpl.java
Line 116: entity.setLastRefreshed(rs.getLong("last_refreshed"));
Line 117:
entity.setFileType(ImageFileType.forValue(rs.getInt("file_type")));
Line 118: return entity;
Line 119: }
Line 120: }
I have the feeling it might have been removed by
http://gerrit.ovirt.org/#/c/13866/
....................................................
File
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/RepoFileMetaDataDAOTest.java
Line 207: assertSame(true, !listOfRepoFiles.isEmpty());
Line 208: assertSame(
Line 209: true,
Line 210: listOfRepoFiles.get(0).getRepoImageId()
Line 211: .equals(newRepoFileMap.getRepoImageId()));
Is this ok?
assertSame(listOfRepoFiles.get(0).getRepoImageId()
newRepoFileMap.getRepoImageId());
Line 212: assertSame(true,
Line 213: listOfRepoFiles.get(0).getLastRefreshed() ==
newRepoFileMap
Line 214: .getLastRefreshed());
Line 215: assertSame(true,
....................................................
File backend/manager/modules/dal/src/test/resources/fixtures.xml
Line 3873:
Line 3874: <table name="repo_file_meta_data">
Line 3875: <column>repo_domain_id</column>
Line 3876: <column>repo_image_id</column>
Line 3877: <column>size</column>
Done
Line 3878: <column>last_refreshed</column>
Line 3879: <column>file_type</column>
Line 3880: <row>
Line 3881: <value>d034f3b2-fb9c-414a-b1be-1e642cfe57ae</value>
--
To view, visit http://gerrit.ovirt.org/13958
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2791fa22820fc021938709f4a921ab4d6213fab7
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
