You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/10/23 19:32:59 UTC
[PR] refactor(aws)!: standardize exported AWS initialisms [iceberg-go]
fallintoplace opened a new pull request, #2310:
URL: https://github.com/apache/iceberg-go/pull/2310
## What
Use `AWS` consistently in exported Go names.
## Why
These names should follow Go initialism conventions before v1.
## Implementation
- Rename the AWS config helpers, Glue options, and CLI profile fields.
- Keep the YAML key and `--aws-profile` flag unchanged.
- Update callers and documentation.
Breaking API: callers must update renamed symbols.
Related: #2062
--
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
Re: [PR] refactor(aws)!: standardize exported AWS initialisms [iceberg-go]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka merged PR #2310:
URL: https://github.com/apache/iceberg-go/pull/2310
--
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
Re: [PR] refactor(aws)!: standardize exported AWS initialisms [iceberg-go]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on code in PR #2310:
URL: https://github.com/apache/iceberg-go/pull/2310#discussion_r4368746865
##########
catalog/rest/sigv4/sigv4_internal_test.go:
##########
@@ -221,13 +221,13 @@ func TestRegisteredBackendEnablesSigV4(t *testing.T) {
assert.NotEmpty(t, gotSHA, "request should carry x-amz-content-sha256")
}
-// TestWithAwsConfigUsesConfiguredRegionService verifies that sigv4.WithAwsConfig
+// TestWithAWSConfigUsesConfiguredRegionService verifies that sigv4.WithAWSConfig
// signs with the region/service from WithSigV4RegionSvc, not the aws.Config's
// own region. Before WithAwsConfig became a signer factory it froze the scope at
// option-construction time, so the natural migration (keep WithSigV4RegionSvc,
// swap rest.WithAwsConfig for sigv4.WithAwsConfig) silently signed for the wrong
Review Comment:
The old `sigv4.WithAwsConfig` name survives in three places: this line, the bare `WithAwsConfig` two lines up, and [`rest_internal_test.go` L2562](https://github.com/apache/iceberg-go/blob/2119b8f7208af06beed76d5e38d24170acc9515d/catalog/rest/rest_internal_test.go#L2562), outside the diff. `rest.WithAwsConfig` can stay, since that was the removed option's name.
```suggestion
// swap rest.WithAwsConfig for sigv4.WithAWSConfig) silently signed for the wrong
```
##########
website/src/configuration.md:
##########
@@ -146,8 +146,8 @@ Invalid values (non-positive waits/retries, or min ≥ max) are ignored and the
Source: [`catalog/glue/options.go`](https://github.com/apache/iceberg-go/blob/main/catalog/glue/options.go).
-- `WithAwsConfig(cfg aws.Config)` - AWS SDK v2 config; respects the AWS default credential chain.
-- `WithAwsProperties(props AwsProperties)` - explicit overrides for region/endpoint/access keys.
+- `WithAWSConfig(cfg aws.Config)` - AWS SDK v2 config; respects the AWS default credential chain.
+- `WithAWSProperties(props AWSProperties)` - explicit overrides for region/endpoint/access keys.
Review Comment:
Since this line changes anyway, could it say what the option does? Only [`toAwsConfig`](https://github.com/apache/iceberg-go/blob/2119b8f7208af06beed76d5e38d24170acc9515d/catalog/glue/glue.go#L97-L142) reads the `glue.region`, endpoint and credential keys, and its only non-test caller is the `catalog.Load` registrar, so `NewCatalog` ignores them, here and on `main`. Could we open an issue for the ignored keys themselves?
```suggestion
- `WithAWSProperties(props AWSProperties)` - catalog properties, such as `glue.id` and the `s3.*` FileIO keys. `NewCatalog` ignores the `glue.region`, endpoint, credential and retry keys; only `catalog.Load` reads them.
```
--
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