Change register to apply - #4
Conversation
pradithya
left a comment
There was a problem hiding this comment.
LGTM.
Need to remember that register is no longer exist and any documentation should refer to it as apply
| * @param otherFeatureGroup | ||
| * @return boolean | ||
| */ | ||
| public boolean eq(FeatureGroupInfo otherFeatureGroup) { |
There was a problem hiding this comment.
Isn't it possible (and perhaps cleaner) to use the built in .equals() method that all Java objects have?
https://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#equals(java.lang.Object)
| EntityInfo entity1 = new EntityInfo(entitySpec); | ||
| entity1.setCreated(Date.from(Instant.ofEpochSecond(1))); | ||
| EntityInfo entity2 = new EntityInfo(entitySpec); | ||
| entity1.setCreated(Date.from(Instant.ofEpochSecond(2))); |
There was a problem hiding this comment.
Is this line correct? Shouldn't it be entity2.setCreated(Date.from(Instant.ofEpochSecond(2)));?
| && oldStoreOpts.equals( | ||
| newStore == null | ||
| ? "" | ||
| : TypeConversion.convertMapToJsonString(newStore.getOptionsMap())); |
There was a problem hiding this comment.
Do we really need to have oldStoreOpts here? Can this also be a method on StorageInfo?
|
Cleaned it up a bit. |
| return storageInfo; | ||
| } else { | ||
| storageInfo = new StorageInfo(spec); | ||
| StorageInfo out = storageInfoRepository.saveAndFlush(storageInfo); |
There was a problem hiding this comment.
Isn't there a more descriptive variable name that we can use instead of out?
Change register API to apply.
If you apply a spec for a resource that doesn't already exist in the system, it will be created. If not, then the resource will be updated to match the new spec provided. Note that only fields without dependencies can be updated; i.e. changes to ids, storage sinks, or value types are not allowed. If you apply a spec that has no changes without throwing an error.
Updated the CLI to reflect these changes as well.