search

Farfetch / kafkaflow-retry-extensions

Kafka Flow Retry Patterns Extensions
https://farfetch.github.io/kafkaflow-retry-extensions/
MIT License
55 stars 7 forks source link

Postgres support #108

Closed alek-github closed 1 year ago

alek-github commented 1 year ago

Description

Added postgres support to kafkaflow-retry-extensions based on the implementation for sql server. It's needed to enable using the package with postgres db. Decided to make it as similar as possible to sql code, to make sure it's aligned to the existing style & practices and does not cause any discrepancies. Further adjustments of extracting shared code could be done in separate PRs, after confirming it with the repo owners. All of the postgres adjustments (comparing to sql code/scripts) can be viewed by skipping first commit

How Has This Been Tested?

Run all unit tests and integration tests locally to make sure that new integration works. New integration is covered same way as sql server

Checklist

Disclaimer

By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement

alek-github commented 1 year ago

Would appreciate Your review @luispfgarces @filipeesch @fernando-a-marins

luispfgarces commented 1 year ago

We need to merge #93 to fix the build pipeline (support for builds with forked repositories).

luispfgarces commented 1 year ago

@alek-talabat the PR #93 was merged, which will fix your build pipeline. Please merge from main branch.

martinhonovais commented 1 year ago

Hi @alek-talabat, First of all, many thanks for your contribution, we are very glad to review it. Everything seemed good to me but I have some considerations:

  1. I feel that we could have a sample using the PostgresSql repo. This could help the PostgresSql users experimenting the repo with not too much effort.
  2. Last week we merged to main a feat that allows the dev to apply a retention policy for queues that have been done. Please take it into consideration in your PR as well. It will help devs to save repo space and money.
  3. Let us know if you did some API's tests using PostgreSql as a repo and if it went well.
gsferreira commented 1 year ago
  • I feel that we could have a sample using the PostgresSql repo. This could help the PostgresSql users experimenting the repo with not too much effort.

@alek-talabat the sample can be a different PR.

And by the way, excellent contribution right here! 👏

luispfgarces commented 1 year ago

@alek-talabat can you please check the build error?

Error: /home/runner/work/kafkaflow-retry-extensions/kafkaflow-retry-extensions/src/KafkaFlow.Retry.Postgres/RetryQueueDataProvider.cs(18,52): error CS0535: 'RetryQueueDataProvider' does not implement interface member 'IRetryDurableQueueRepositoryProvider.DeleteQueuesAsync(DeleteQueuesInput)' [/home/runner/work/kafkaflow-retry-extensions/kafkaflow-retry-extensions/src/KafkaFlow.Retry.Postgres/KafkaFlow.Retry.Postgres.csproj]

I think that the pull request that went Live before yours created a new method on the repository interface, which yours does not have (because you implemented it before this change).

Thank you! 🙏

alek-github commented 1 year ago

Sorry for the delay, but changes from the most recent master are already included. Would appreciate Your reviews @martinhonovais @gsferreira to have this one closed