chore: Split powertools-idempotency module to sub-modules

[eldimi]

Issue #1467

Description of changes:

The idempotency module is split to sub-modules so that the various implementations (currently only dynamodb), are provided as pluggable extensions

Checklist

• Meet tenets criteria
• Update tests
• Update docs
• PR title follows conventional commit semantics

Breaking change checklist

RFC issue #:

• Migration process documented
• Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[jeromevdl]

@scottgerring we had a quick chat with @eldimi about the structure of the project which is a bit different from what you did for parameters. Not sure if we should align or not as it's slightly different. Several implementations of parameters VS a core + a persistence layer.

Actually, I'm not sure if the customer should have one or 2 dependencies. My view is that we should have:

• a core module (idempotency-core), just like jackson-core, java-sdk-core, jmespath-core, ... (instead of common).
• a module per persistence technology (idempotency-persistence-dynamodb ?). It's not really an implementation of the core (partially let say), but it complements the core.

Also looking at the packages, I would rather have software.amazon.lambda.powertools.idempotency.persistence.dynamodb.* than software.amazon.lambda.powertools.idempotency.dynamodb.persistence.*. Dynamodb is a subpackage of persistence, not a parent one.

[eldimi]

@scottgerring we had a quick chat with @eldimi about the structure of the project which is a bit different from what you did for parameters. Not sure if we should align or not as it's slightly different. Several implementations of parameters VS a core + a persistence layer.

Actually, I'm not sure if the customer should have one or 2 dependencies. My view is that we should have:

• a core module (idempotency-core), just like jackson-core, java-sdk-core, jmespath-core, ... (instead of common).
• a module per persistence technology (idempotency-persistence-dynamodb ?). It's not really an implementation of the core (partially let say), but it complements the core.

Also looking at the packages, I would rather have software.amazon.lambda.powertools.idempotency.persistence.dynamodb.* than software.amazon.lambda.powertools.idempotency.dynamodb.persistence.*. Dynamodb is a subpackage of persistence, not a parent one.

Good point! I will adapt naming and package structure

[scottgerring]

@scottgerring we had a quick chat with @eldimi about the structure of the project which is a bit different from what you did for parameters. Not sure if we should align or not as it's slightly different. Several implementations of parameters VS a core + a persistence layer.

Parameters don't share a common interface on the configuration / injection side - you can't generically provide a @Param(...) because of this. By way of example, the fields you need to inject an AppConfig param are very different to a DynamoDB one due to the nature of the underlying services. Trying to stretch an abstraction around these variances in v1 is a big part of why params v1 is quite messy on the construction side. On the other hand, In cases where we legitimately have a common interface then it makes sense to keep that in the -core module.

Thinking harder, in the idempotency case we are trying to make an abstraction without more than one example of it. I think we should implement one other store - e.g. RDS - so that we know that the way we are trying to shape this is meaningful.

In terms of extra "inheritance" through the poms, I don't have strong opinions. Is there some stuff that is grouped up here that logically belongs there? In general, let's be consistent unless there is a compelling reason not to.

@jeromevdl what do you think?

Actually, I'm not sure if the customer should have one or 2 dependencies. My view is that we should have:

* a core module (`idempotency-core`), just like `jackson-core`, `java-sdk-core`, `jmespath-core`, ... (instead of `common`).

* a module per persistence technology (`idempotency-persistence-dynamodb` ?). It's not really an implementation of the core (partially let say), but it complements the core.

Also looking at the packages, I would rather have software.amazon.lambda.powertools.idempotency.persistence.dynamodb.* than software.amazon.lambda.powertools.idempotency.dynamodb.persistence.*. Dynamodb is a subpackage of persistence, not a parent one.

In this case, ACK s/common/core, we just need to be careful with our naming with regard to the other powertools implementations. This is why powertools-core has been renamed in v2 - because it doesn't contain the powertools core functionality as powertools thinks of it in general across languages. I think it's fine here, it's just worth keeping in mind that there's a bit of a thing with the java naming and the PT naming here.

[jeromevdl]

Thinking harder, in the idempotency case we are trying to make an abstraction without more than one example of it. I think we should implement one other store - e.g. RDS - so that we know that the way we are trying to shape this is meaningful.

I agree we should test our interface with another implementation. I think redis makes more sense for this kind of usage. It is one of the most used/appreciated "database". @eldimi, do you feel comfortable / do you have time to add another implementation with redis (not elasticache, really the redis implementation, so passing the redis connection url and so on)? We could even try that with https://upstash.com/.

In terms of extra "inheritance" through the poms, I don't have strong opinions. Is there some stuff that is grouped up here that logically belongs there? In general, let's be consistent unless there is a compelling reason not to.

I don't have a strong opinion either. I think what you did is ok (parent pom, core module, dynamodb module, redis module) and dynamodb/redis depend on core.

[eldimi]

Thinking harder, in the idempotency case we are trying to make an abstraction without more than one example of it. I think we should implement one other store - e.g. RDS - so that we know that the way we are trying to shape this is meaningful.

I agree we should test our interface with another implementation. I think redis makes more sense for this kind of usage. It is one of the most used/appreciated "database". @eldimi, do you feel comfortable / do you have time to add another implementation with redis (not elasticache, really the redis implementation, so passing the redis connection url and so on)? We could even try that with https://upstash.com/.

I had the same feeling while splitiing to modules. That we are missing a second example. I can do the redis impl, but I think we should do this in a new PR. So, that this closes soon, right?

[scottgerring]

Thinking harder, in the idempotency case we are trying to make an abstraction without more than one example of it. I think we should implement one other store - e.g. RDS - so that we know that the way we are trying to shape this is meaningful.

I agree we should test our interface with another implementation. I think redis makes more sense for this kind of usage. It is one of the most used/appreciated "database". @eldimi, do you feel comfortable / do you have time to add another implementation with redis (not elasticache, really the redis implementation, so passing the redis connection url and so on)? We could even try that with https://upstash.com/.

I had the same feeling while splitiing to modules. That we are missing a second example. I can do the redis impl, but I think we should do this in a new PR. So, that this closes soon, right?

I think it would be a nice way to prove the abstraction and would be nice to do in the same PR. If you're just going to go and do the next impl immediately after, same branch? If you'd like to see progress and will come back to this later, probably we can be a bit more flexible - v2 is still not a "production grade" branch so changing the interface after if we need to isn't so tragic.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[eldimi]

work will continue in #1513. See comment.