elek commented on a change in pull request #2136:
URL: https://github.com/apache/ozone/pull/2136#discussion_r635094043
##########
File path:
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/keys/CopyKeyHandler.java
##########
@@ -82,17 +84,19 @@ protected void execute(OzoneClient client, OzoneAddress
address)
OzoneVolume vol = client.getObjectStore().getVolume(volumeName);
OzoneBucket bucket = vol.getBucket(bucketName);
- if (replicationFactor == null) {
- replicationFactor = ReplicationFactor.valueOf(
- getConf().getInt(OZONE_REPLICATION, OZONE_REPLICATION_DEFAULT));
- }
-
if (replicationType == null) {
replicationType = ReplicationType.valueOf(
- getConf().get(OZONE_REPLICATION_TYPE,
- OZONE_REPLICATION_TYPE_DEFAULT));
+ getConf()
+ .get(OZONE_REPLICATION_TYPE, OZONE_REPLICATION_TYPE_DEFAULT));
}
+ if (replication == null) {
Review comment:
> ENUMS are not dangerous every time when we transfer them over the
network.
Thanks the comment @mukul1987 and @arp7
As we discussed during the EC meeting it's NOT about protobuf changes. It's
a client side only change.
In general: we need multiple type of replication configs. For example:
```
new RatisReplicationConfig(ONE)
new RatisReplicationConfig(THREE)
new StandaloneReplicationConfig(ONE)
new StandaloneReplicationConfig(THREE)
new ECReplicationConfig(2,1,XOR)
new ECReplicationConfig(4,3,RS)
new ECReplicationConfig(6,4,RS)
new ECReplicationConfig(10,1,RS)
```
All of these configs are strongly typed both in Java code (OM/SCM) and
protobuf. ONE/THREE are enums as before.
The only problem here is that Enum types / replication definitions **depend
on the replication config type**. EC may need different enum values. First we
need to choose the replication config based on the type (EC/RATIS/STANDLAONE)
and **after that** we can ask the selected `ReplicationConfig` to parse the
string to `Enum`. This is what we do in this patch.
It's a separated discussion if we need to validate the input string in
`ECReplicationConfig` based on predefined enums or based on smart validation
rules. Both can work, and please add your opinion to
https://issues.apache.org/jira/browse/HDDS-5247
> The bigger issue is a lack of discipline in thinking through compatibility
concerns.
Thanks the feedback @arp7
Can you please share your compatibility concerns?
This is a client side change and as you can see none of the smoketests are
changed, so we can assume it's fully compatible. `ozone.replication` is also
changed, but it accepts both integer and string existing values work as before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
