amogh-jahagirdar commented on code in PR #7548:
URL: https://github.com/apache/iceberg/pull/7548#discussion_r1187856959
##########
docs/aws.md:
##########
@@ -198,8 +198,8 @@ With optimistic locking, each table has a version id.
If users retrieve the table metadata, Iceberg records the version id of that
table.
Users can update the table as long as the version ID on the server side
remains unchanged.
If there is a version mismatch, it means that someone else has modified the
table before you did.
-The update attempt fails, because you have a stale version of the table.
-If this happens, Iceberg refreshes the metadata and checks if there might be
potential conflict.
+The update attempt fails because you have a stale version of the table.
+If this happens, Iceberg refreshes the metadata and checks if there might be a
potential conflict.
Review Comment:
We can further shorten this to `checks if there is a conflict`
##########
docs/aws.md:
##########
@@ -611,10 +611,10 @@ Apache HTTP Client has the following configurable
properties:
| http-client.apache.connection-acquisition-timeout-ms | null
| An optional [connection acquisition
timeout](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#connectionAcquisitionTimeout(java.time.Duration))
in milliseconds |
| http-client.apache.connection-max-idle-time-ms | null
| An optional [connection max idle
timeout](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#connectionMaxIdleTime(java.time.Duration))
in milliseconds |
| http-client.apache.connection-time-to-live-ms | null
| An optional [connection time to
live](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#connectionTimeToLive(java.time.Duration))
in milliseconds |
-| http-client.apache.expect-continue-enabled | null, disabled by
default | An optional `true/false` setting that decide whether to enable
[expect
continue](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#expectContinueEnabled(java.lang.Boolean))
|
+| http-client.apache.expect-continue-enabled | null, disabled by
default | An optional `true/false` setting that decides whether to enable
[expect
continue](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#expectContinueEnabled(java.lang.Boolean))
|
| http-client.apache.max-connections | null
| An optional [max
connections](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#maxConnections(java.lang.Integer))
in integer |
-| http-client.apache.tcp-keep-alive-enabled | null, disabled by
default | An optional `true/false` setting that decide whether to enable [tcp
keep
alive](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#tcpKeepAlive(java.lang.Boolean))
|
-| http-client.apache.use-idle-connection-reaper-enabled | null, enabled by
default | An optional `true/false` setting that decide whether to [use idle
connection
reaper](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#useIdleConnectionReaper(java.lang.Boolean))
|
+| http-client.apache.tcp-keep-alive-enabled | null, disabled by
default | An optional `true/false` setting that decides whether to enable [tcp
keep
alive](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#tcpKeepAlive(java.lang.Boolean))
|
+| http-client.apache.use-idle-connection-reaper-enabled | null, enabled by
default | An optional `true/false` setting that decides whether to [use idle
connection
reaper](https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/http/apache/ApacheHttpClient.Builder.html#useIdleConnectionReaper(java.lang.Boolean))
|
Review Comment:
I think we can remove the "decides", these settings aren't sentient :) .
How about `An optional `true/false` setting that controls whether [use idle
connection reaper] is used`
##########
docs/aws.md:
##########
@@ -555,7 +555,7 @@ This client factory has the following configurable catalog
properties:
| client.assume-role.arn | null, requires user input
| ARN of the role to assume, e.g. arn:aws:iam::123456789:role/myRoleToAssume |
| client.assume-role.region | null, requires user input
| All AWS clients except the STS client will use the given region instead of
the default region chain |
| client.assume-role.external-id | null
| An optional [external
ID](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create_for-user_externalid.html)
|
-| client.assume-role.timeout-sec | 1 hour
| Timeout of each assume role session. At the end of the timeout, a new set of
role session credentials will be fetched through a STS client. |
+| client.assume-role.timeout-sec | 1 hour
| Timeout of each assume role session. At the end of the timeout, a new set of
role session credentials will be fetched through an STS client. |
Review Comment:
I think we can leave this unchanged. "a STS client" is correct
--
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]
