jx2lee commented on code in PR #2012:
URL: https://github.com/apache/iceberg-go/pull/2012#discussion_r4025183479
##########
catalog/glue/glue_test.go:
##########
@@ -1029,6 +1029,35 @@ func TestGlueCreateNamespace(t *testing.T) {
assert.NoError(err)
}
+func TestGlueCreateNamespaceAlreadyExists(t *testing.T) {
+ assert := require.New(t)
+
+ mockGlueSvc := &mockGlueClient{}
+
+ mockGlueSvc.On("CreateDatabase", mock.Anything,
&glue.CreateDatabaseInput{
+ DatabaseInput: &types.DatabaseInput{
+ Name: aws.String("test_namespace"),
+ Description: aws.String("Test Description"),
+ LocationUri: aws.String("s3://test-location"),
+ Parameters: map[string]string{},
+ },
+ }, mock.Anything).Return(&glue.CreateDatabaseOutput{},
&types.AlreadyExistsException{
+ Message: aws.String("Database already exists"),
+ }).Once()
Review Comment:
👍🏽 I added to missing AssertExceptions. Nice catch, Thanks !
##########
catalog/glue/glue.go:
##########
@@ -653,6 +653,11 @@ func (c *Catalog) CreateNamespace(ctx context.Context,
namespace table.Identifie
DatabaseInput: constructDatabaseInput(database, props),
})
if err != nil {
+ var alreadyExistsErr *types.AlreadyExistsException
+ if errors.As(err, &alreadyExistsErr) {
+ return fmt.Errorf("failed to create database %s: %w",
database, catalog.ErrNamespaceAlreadyExists)
Review Comment:
👍🏽 Three methods are covered. Thanks to suggestion !
##########
catalog/glue/glue_test.go:
##########
@@ -1029,6 +1029,35 @@ func TestGlueCreateNamespace(t *testing.T) {
assert.NoError(err)
}
+func TestGlueCreateNamespaceAlreadyExists(t *testing.T) {
+ assert := require.New(t)
+
+ mockGlueSvc := &mockGlueClient{}
+
+ mockGlueSvc.On("CreateDatabase", mock.Anything,
&glue.CreateDatabaseInput{
+ DatabaseInput: &types.DatabaseInput{
+ Name: aws.String("test_namespace"),
+ Description: aws.String("Test Description"),
+ LocationUri: aws.String("s3://test-location"),
+ Parameters: map[string]string{},
+ },
+ }, mock.Anything).Return(&glue.CreateDatabaseOutput{},
&types.AlreadyExistsException{
+ Message: aws.String("Database already exists"),
+ }).Once()
+
+ glueCatalog := &Catalog{
+ glueSvc: mockGlueSvc,
+ }
+
+ props := map[string]string{
+ "comment": "Test Description",
+ PropsKeyLocation: "s3://test-location",
+ }
+
+ err := glueCatalog.CreateNamespace(context.TODO(),
DatabaseIdentifier("test_namespace"), props)
+ assert.ErrorIs(err, catalog.ErrNamespaceAlreadyExists)
Review Comment:
👍🏽 Thanks to suggestion. added new test, including ErrContains assertion.
--
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]
