You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "amogh-jahagirdar (via GitHub)" <gi...@apache.org> on 2023/05/08 20:17:38 UTC
[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #7548: Fix Typo and Polish in aws.md
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org