mdedetrich commented on code in PR #1246:
URL: https://github.com/apache/pekko-connectors/pull/1246#discussion_r2529753421


##########
s3/src/main/resources/reference.conf:
##########
@@ -125,4 +125,20 @@ pekko.connectors.s3 {
 
   # Add signature headers to requests when aws.credentials.provider is anon
   sign-anonymous-requests = true
+
+  additional-allowed-headers {

Review Comment:
   Can we add a comment here so readers have context?



##########
s3/src/test/scala/org/apache/pekko/stream/connectors/s3/S3HeadersSpec.scala:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.pekko.stream.connectors.s3
+
+import org.scalatest.flatspec.AnyFlatSpecLike
+import org.scalatest.matchers.should.Matchers
+import org.apache.pekko.stream.connectors.s3.impl.S3Request
+import com.typesafe.config.ConfigFactory
+import scala.reflect.ClassTag
+import org.apache.pekko.stream.connectors.s3.impl.GetObject

Review Comment:
   These imports need to be cleaned but I will do this by committing on your PR



##########
s3/src/main/resources/reference.conf:
##########
@@ -125,4 +125,20 @@ pekko.connectors.s3 {
 
   # Add signature headers to requests when aws.credentials.provider is anon
   sign-anonymous-requests = true
+
+  additional-allowed-headers {
+    GetObject = []

Review Comment:
   I might be missing something, but are these the right defaults? It feels 
like by default there should be some entries here (since you are testing them) 
and if someone doesn't want these additional allowed headers then they can 
override it in their own `.conf` with an empty list.



-- 
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]

Reply via email to