Archival patch (Checkstyles not handled) #45
Conversation
…scheduler (time based)
| if (result != 0) { | ||
| return result; | ||
| } | ||
|
|
There was a problem hiding this comment.
This could potentially throw NPE since tags could be null. Instead, you can use null-safe comparison provided by ObjectUtils class -
result = ObjectUtils.compare(this.tags, o.tags);
There was a problem hiding this comment.
Agree with @deepakbarr @pragya-mittal can you please make the relevant change and update the pull request?
…o()" This reverts commit db094ea.
pragya-mittal
changed the title
| <xs:complexType name="archival-stage"> | ||
| <xs:annotation> | ||
| <xs:documentation> | ||
| Archival stage is the new way to define archival for a feed using feed lifecycle feature. Archival |
There was a problem hiding this comment.
remove new... It will get old at some point :-).
In my mind, the key difference between replication and archival is that, in replication, it is a data copy to the same type of storage. With archival, the destination storage can be different and it is a data move. Bring that difference out in the documentation.
| </xs:annotation> | ||
| <xs:all> | ||
| <xs:element type="non-empty-string" name="policy" minOccurs="0" maxOccurs="1"></xs:element> | ||
| <xs:element type="xs:string" name="deleteFromSource" minOccurs="0"></xs:element> |
There was a problem hiding this comment.
This should be a boolean. missing maxOccurs=1
| <xs:element type="non-empty-string" name="policy" minOccurs="0" maxOccurs="1"></xs:element> | ||
| <xs:element type="xs:string" name="deleteFromSource" minOccurs="0"></xs:element> | ||
| <xs:element type="xs:string" name="queue" minOccurs="0" maxOccurs="1"></xs:element> | ||
| <xs:element type="location" name="location" minOccurs="1"></xs:element> |
There was a problem hiding this comment.
Do we allow more than one location. If not? maxOccurs should be present.
| import org.apache.falcon.entity.v0.feed.RetentionStage; | ||
| import org.apache.falcon.entity.v0.feed.Sla; | ||
| import org.apache.falcon.entity.v0.feed.Validity; | ||
| import org.apache.falcon.entity.v0.feed.*; |
There was a problem hiding this comment.
Has been a conscious choice not to use wild card imports, so far. Lets not deviate from that.
| policy = StringUtils.isBlank(policy) | ||
| ? FeedLifecycleStage.RETENTION.getDefaultPolicyName() : policy; | ||
| result.add(policy); | ||
| String policy = ""; |
There was a problem hiding this comment.
getPolicies method : rather than return the generic list of all policies across stages, please have separate methods. Clubbing policies across lifecycle stages does not make sense.
| ArchivalStage archivalStage = getArchivalStage(feed, clusterName); | ||
| if (archivalStage != null) { | ||
| Location location = archivalStage.getLocation(); | ||
| if ((location != null) && (location.getPath() != null)) { |
| if ((FeedHelper.getRetentionStage(feed, cluster.getName()) != null) || (FeedHelper.getArchivalStage(feed, cluster.getName()) != null)) { | ||
| validateRetentionStage(feed, cluster); | ||
| validateArchivalStage(feed, cluster); | ||
| for (String policyName : FeedHelper.getPolicies(feed, cluster.getName())) { |
There was a problem hiding this comment.
Once you split the getPolicies method. Use the appropriate ones inside validateRetentionStage and validateArchivalStage.
| private void validateArchivalStage(Feed feed, Cluster cluster) throws FalconException { | ||
| if (FeedHelper.getArchivalStage(feed, cluster.getName()) != null) { | ||
| if (FeedHelper.getArchivalStage(feed, cluster.getName()).getLocation().getPath().isEmpty()) { | ||
| throw new ValidationException("Location path cannot be empty."); |
There was a problem hiding this comment.
A little more details on the message like the mention of archival stage for the given cluster will be useful.
| /** | ||
| * Enum for valid lifecycle stages for the feed. | ||
| */ | ||
| public enum FeedLifecycleStage { |
There was a problem hiding this comment.
This class needs to be re-visited. A common set of policies for all lifecycles and a default policy across stages does not make sense to me. May be have a base class and child classes.
pragya-mittal
changed the title
…e#45) * FALCON-298. Feed update with replication delay creates holes * rebasing from master * FALCON-2097. Adding UT to the new method for getting next instance time with Delay. * reverting last line changes made * FALCON-2319. Falcon Build failure fix for enunciate * FALCON-2324 Retry, Latererun and JobLog mover services not working with kerberos enabled
4 participants
No description provided.