Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make DynamoDb Clients Standalone #772

Closed
wants to merge 1 commit into from
Closed

Conversation

driverpt
Copy link

@driverpt driverpt commented Apr 20, 2023

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Issue: #768 #769

DynamoDb Clients are now initialized Standalone

💡 Motivation and Context

We do not use Spring Data and just wanted simple DynamoDb Initialization

💚 How did you test it?

Integration Tests

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated reference documentation to reflect the change
  • All tests passing
  • No breaking changes

🔮 Next steps

@github-actions github-actions bot added the component: dynamodb DynamoDB integration related issue label Apr 20, 2023
Copy link
Contributor

@maciejwalkowiak maciejwalkowiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you probably noticed, tests are failing because autoconfiguration classes need to be updated with correct ConditionalOnBeans.

Also, each autoconfiguration class needs a separate test class that verifies behavior for cases when beans are not there or classes are not there.

Copy link
Member

@MatejNedic MatejNedic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @driverpt ,
Thanks on PR.
Few changes regarding autoconfiguration classes.

@driverpt
Copy link
Author

@maciejwalkowiak @MatejNedic @tomazfernandes , can you please re-review?

Thanks

Copy link
Member

@MatejNedic MatejNedic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @driverpt ,

Few more changes and we should be good.

Check docs on @Conditional why it should be applied to static class rather than the @Bean method directly.

Also, don't forget to add yourself under author list on top of class :)

return DynamoDbEnhancedAsyncClient.builder().dynamoDbClient(dynamoDbClient).build();
}

@ConditionalOnClass(DynamoDbTemplate.class)
@ConditionalOnMissingBean(DynamoDbOperations.class)
@Bean
public DynamoDbTemplate dynamoDBTemplate(DynamoDbProperties properties,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's wrap this into a static class for example:

	@ConditionalOnClass(name = "io.awspring.cloud.dynamodb.DynamoDbOperations")
	@Configuration(proxyBeanMethods = false)
	static class DynamoDBTemplateConfiguration {
		
		@ConditionalOnMissingBean(DynamoDbOperations.class)
		@Bean
		public DynamoDbTemplate dynamoDBTemplate(DynamoDbProperties properties,
												 DynamoDbEnhancedClient dynamoDbEnhancedClient, Optional<DynamoDbTableSchemaResolver> tableSchemaResolver,
												 Optional<DynamoDbTableNameResolver> tableNameResolver) {
			DynamoDbTableSchemaResolver tableSchemaRes = tableSchemaResolver
				.orElseGet(DefaultDynamoDbTableSchemaResolver::new);

			DynamoDbTableNameResolver tableNameRes = tableNameResolver
				.orElseGet(() -> new DefaultDynamoDbTableNameResolver(properties.getTablePrefix()));
			return new DynamoDbTemplate(dynamoDbEnhancedClient, tableSchemaRes, tableNameRes);
		}
	}

@ConditionalOnMissingBean
@ConditionalOnClass(DynamoDbEnhancedAsyncClient.class)
@Bean
public DynamoDbEnhancedAsyncClient dynamoDbEnhancedAsyncClient(DynamoDbAsyncClient dynamoDbClient) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same like with DynamoDBTemplateConfiguration should be done.

@ConditionalOnMissingBean
@ConditionalOnClass(DynamoDbEnhancedClient.class)
@Bean
public DynamoDbEnhancedClient dynamoDbEnhancedClient(DynamoDbClient dynamoDbClient) {
Copy link
Member

@MatejNedic MatejNedic May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same like with DynamoDBTemplateConfiguration should be done.

@driverpt driverpt closed this Sep 22, 2023
@github-actions github-actions bot added component: cloudwatch CloudWatch integration related issue component: core Core functionality related issue component: parameter-store Parameter Store integration related issue component: s3 S3 integration related issue component: secrets-manager Secrets Manager integration related issue component: ses SES integration related issue component: sns SNS integration related issue component: sqs SQS integration related issue type: dependency-upgrade Dependency version bump type: documentation Documentation or Samples related issue labels Sep 22, 2023
@driverpt
Copy link
Author

Can this be reopened? I have no idea why it was closed

@driverpt driverpt mentioned this pull request Sep 22, 2023
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: cloudwatch CloudWatch integration related issue component: core Core functionality related issue component: dynamodb DynamoDB integration related issue component: parameter-store Parameter Store integration related issue component: s3 S3 integration related issue component: secrets-manager Secrets Manager integration related issue component: ses SES integration related issue component: sns SNS integration related issue component: sqs SQS integration related issue type: dependency-upgrade Dependency version bump type: documentation Documentation or Samples related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants