gardenia commented on code in PR #10318: URL: https://github.com/apache/ozone/pull/10318#discussion_r3277094744
########## hadoop-ozone/ozone-manager-plugins/src/main/java/org/apache/hadoop/ozone/om/eventlistener/s3/S3EventNotification.java: ########## @@ -0,0 +1,671 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om.eventlistener.s3; + +/* copy of + * software.amazon.awssdk.eventnotifications.s3.model.S3EventNotification + * class taken from AWS SDK (1.x) with minor changes for build issues + * and removed usage of unnecessary AWS specific extension entiies: Review Comment: I explored using the SDK class originally, but ended up going with a fork for a few reasons: 1. Classpath bloat: OM doesn't currently use the AWS SDK at runtime. That class has internal dependencies on SDK utility classes such as SdkHttpUtils and their own Jackson wrapper. Using it would force us to pull in the wole SDK core and its transitive dependencies (Joda-Time, older HttpComponents, etc.), which we want to avoid in the core server. 2. Extensibility: We need to add Ozone-specific metadata (like isDirectory or renameFromKey). The SDK class is a fixed contract—to add our custom fields, we’d have to use hacky Jackson wrappers or 'Mix-ins' to override its behavior from the outside. 3. The SDK v1 class is hard-coded to Joda-Time. By forking, I was able to switch everything to native OffsetDateTime, so we don't have to carry legacy date-library baggage in our pom.xml. 4. The SDK v2 version of the same class wasn't any better. It is more rigid, uses immutable builders and is messy to add bespoke fields. If there is a consensus of opinion that we need to avoid this somehow I am open to revisiting but those are the reasons I went with the fork. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
