-
Notifications
You must be signed in to change notification settings - Fork 5
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
Upgrade to Panama.Core v6 #22
Comments
Here is how the Notes:
|
@mrogunlana |
Yes that's correct, these test require a db (specifically the batch and
mysql test cases).
We can revisit once core v6 is stable
…On Wed, Feb 15, 2023, 8:25 AM CodeSmith ***@***.***> wrote:
@mrogunlana <https://github.com/mrogunlana>
On tryiing to run tests, some currently fail. I'm guessing they are db
related and can wait till we finish the DI
—
Reply to this email directly, view it on GitHub
<#22 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACZEOAIG264DXNS26P7B5ALWXTKNLANCNFSM6AAAAAAUZMPY2U>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Alright, |
So, Reading and looking through the package. |
I think the above assumption is wrong since we are loading the assemblies outside the package, we just need to replace the dependency on autofac right? |
I believe having a service locator would still be valid in some cases so replacing the dependency on autofac would be the right move. The Handler itself should depend on the DI/IoC .NET container in its constructor though (e.g. instead of referencing the service locator). Does this make sense? |
Use this branch for the updates: https://github.com/mrogunlana/Panama.Core/tree/panama-core-v6 |
@mrogunlana Can you take a look at this commit 312d161, I've tried to refactor the handler class. Made some design decisions also alongst the process. Let me know what you think before I proceed. |
Great work so far, left some comments on the commit: 312d161#commitcomment-100953812 |
Also updated the service registrar #22
see latest commit on branch 51c140e I moved all files that will make the v6 release in the root directory with the Here is a design consideration for you: how can we build out the execution workflow of the handler in a extensible fashion? If you examine https://github.com/mrogunlana/Panama.Core/blob/master/Panama/Commands/Handlers/Handler.cs#L285 you'll see that the current Here is the update based on this consideration: 8852f0d I think this should be a good base version of the Handler for v6. Please clean up the repository e.g., remove all unused projects files and the only project(s) that should remain are:
All directories in Panama.Core should be removed except the following folders:
Update all tests to point to Panama.Core.Handler and the objects in the Panama.Core namespace. Also please thoroughly test the new Handler to make sure the Invoke function works as expected. One nuance I made to unify the ICommand, IRollback, IValidate, IQuery interfaces is that they all inherit from IExecute. The breaking change is to the IValidate where the old version had a Once you update all references and clean up the project (and the build is stable 😄) we can move on the adding the IPublish, ISubscribe, IMessage, IBroker to the library which should all the features we need to rollout. Thoughts? |
So, I'm trying to update the classes in the test project and I can see that some classes have references to removed projects like sql etc. I'm planning to remove those tests for now. We can add them afterwards when we reach those parts of the project. |
@AbdulmueezEmiola cleaned up obsolete projects and added a few things in this commit (build broken): d8ef37e |
@mrogunlana Alright, I'm pulling now |
@mrogunlana How should we proceed with the test project, are we also dropping them for now? |
Let's keep the test project, I'm depending on you to resolve the compile errors and update the test project with the proper references. Take a good look at the handler and its contract. I did some pseudo fancy coding to abstract the invocation function from the handler but it may not work. The idea is that I'd like for the handler to be as simple as possible and provide an abstraction around what the handler depends on e.g., the strategy it uses to execute its manifest. Maybe you could refine the rough idea I put together |
@AbdulmueezEmiola here is an example of where named/keyed services: how would we go about doing this in a more "native" dotnet DI way e.g., is it possible to resolve a keyed/named service using constructor injection here? |
So, based on looking at the previous commits.
I'm guessing *nameof(Base64Encryptor) *is the key and we need to resolve
the service based on it.
…On Tue, Feb 28, 2023 at 4:35 AM Diran Ogunlana ***@***.***> wrote:
@AbdulmueezEmiola <https://github.com/AbdulmueezEmiola> here is an
example of where named/keyed services:
https://github.com/mrogunlana/Panama.Core/blob/b0adb4d5036c6bc855bd0affec182384fcd19c3f/Panama.Core.CDC.MySQL/LogTailingProcessor.cs#L26
how would we go about doing this in a more "native" dotnet DI way e.g., is
it possible to resolve a keyed/named service using constructor injection
here?
—
Reply to this email directly, view it on GitHub
<#22 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGFBLDY7JBCDWOWACKBNRZDWZVI4XANCNFSM6AAAAAAUZMPY2U>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
About to take a look at the sql related panama projects and refactor it.
On Tue, Feb 28, 2023 at 5:13 PM abdulmueez emiola <
***@***.***> wrote:
… So, based on looking at the previous commits.
I'm guessing *nameof(Base64Encryptor) *is the key and we need to resolve
the service based on it.
On Tue, Feb 28, 2023 at 4:35 AM Diran Ogunlana ***@***.***>
wrote:
> @AbdulmueezEmiola <https://github.com/AbdulmueezEmiola> here is an
> example of where named/keyed services:
>
> https://github.com/mrogunlana/Panama.Core/blob/b0adb4d5036c6bc855bd0affec182384fcd19c3f/Panama.Core.CDC.MySQL/LogTailingProcessor.cs#L26
>
> how would we go about doing this in a more "native" dotnet DI way e.g.,
> is it possible to resolve a keyed/named service using constructor injection
> here?
>
> —
> Reply to this email directly, view it on GitHub
> <#22 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGFBLDY7JBCDWOWACKBNRZDWZVI4XANCNFSM6AAAAAAUZMPY2U>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
Also, Can you give a brief breakdown of your thoughts for the remaining
packages. Understanding your intuition will help me better understand where
things can be optimized.
On Tue, Feb 28, 2023 at 5:17 PM abdulmueez emiola <
***@***.***> wrote:
… About to take a look at the sql related panama projects and refactor it.
On Tue, Feb 28, 2023 at 5:13 PM abdulmueez emiola <
***@***.***> wrote:
> So, based on looking at the previous commits.
> I'm guessing *nameof(Base64Encryptor) *is the key and we need to resolve
> the service based on it.
>
> On Tue, Feb 28, 2023 at 4:35 AM Diran Ogunlana ***@***.***>
> wrote:
>
>> @AbdulmueezEmiola <https://github.com/AbdulmueezEmiola> here is an
>> example of where named/keyed services:
>>
>> https://github.com/mrogunlana/Panama.Core/blob/b0adb4d5036c6bc855bd0affec182384fcd19c3f/Panama.Core.CDC.MySQL/LogTailingProcessor.cs#L26
>>
>> how would we go about doing this in a more "native" dotnet DI way e.g.,
>> is it possible to resolve a keyed/named service using constructor injection
>> here?
>>
>> —
>> Reply to this email directly, view it on GitHub
>> <#22 (comment)>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/AGFBLDY7JBCDWOWACKBNRZDWZVI4XANCNFSM6AAAAAAUZMPY2U>
>> .
>> You are receiving this because you were mentioned.Message ID:
>> ***@***.***>
>>
>
|
@AbdulmueezEmiola
I'm still in the process of filling out the high level details of the The Does this give you a working idea of where I'm going with the CDC.* namespace(s)? Think of it as a separate worker process that does the lower level scaffolding to guarantee that a message is published and/or received from a broker. All of this low-level wiring will be obfuscated from the developer when publishing a message from the context of a command. |
I think I get the hang of the base CDC package, I have a few ideas on the design which I'll try out and push for you to check. I also get the gist of the sql package also, but want to work on the base package first |
Great, check out the project dotnetcore/cap on github for ideas as its the inspiration for Panama.Core/CDC The remaining This is the container I'm using for the Panama.Core.CDC.MySQL package https://github.com/mrogunlana/Panama.Core/blob/panama-core-v6/Containers/mysql-8/scripts/master.sql for the I'm thinking we want to add an abstraction around https://github.com/mrogunlana/Panama.Core/blob/cab7b9fe975c7264bfd204102e6f479bf3d0e5f5/Panama.Core.CDC.MySQL/Processors/LogTailingProcessor.cs#L81 where it calls the We also have to decide/design The database tables are:
Hopefully this give a better context of the design |
Also, in the dotnetcore/cap package their published table has a status of |
Worked on this #22 (comment) earlier today, just pushing it now, I'll check out the project and your explanation now |
I've moved towards having a |
I'm thinking the handler would look like this in
commands would look something like this in v6:
|
From here #22 (comment)
|
To support multiple brokers, I think we need to separate the outbox and published table. That way, we can easily track the status of each of the brokers |
the dotnetcore/cap project implements an You'll also notice that I've added Here's my thoughts on:
We'll track or identify which broker owns which message in the Here's my thoughts on:
The publish inside the These are all great questions, hopefully it's making sense now? Let me know. If you want to jump in, I'm in the process of implementing the dispatcher and bus and would like to see where you take it. Feel free to move stuff around, delete/add stuff etc. the dotnetcore/cap project is a good rubric for the event bus implementation, but I know we can do better 😃 |
Thinking a little more on adding the We could set the |
What if we made the architectural decision of supporting one Given that the handler would have an ambient transaction, in the event of an exception, the transaction would rollback thereby undoing the database interactions of the command(s) Here is what our
OR:
We could also make the decision to allow developers to queue messages to brokers that get published eventually if the transaction commits internally successfully. I'm still thinking through how exceptions will bubble up given that the transaction will commit downstream of the command performing the database operation. |
we could clean the above command up more by moving the publish events within their own
|
Other idea for handler:
where the
Let me know which of these you believe would be more palatable for developers. |
I'm leaning more toward command design where
Keep in mind, developers can still inject dependencies via constructor of the command to get the same result:
I'm liking this version of the handler stack as opposed to the expression based handler for simplicity sake:
|
I do like the concept of expression based validators though; but things can get tricky in handler scenarios using collections -- but I guess if the handler is type specific e.g.
the handler would be "smart" enough to determine that there is a list and iterate/validate. Maybe we can roll out attribute and expression based validators in a later version after the initial |
Also note, if we introduce an Just having an |
About the 2 structures here, the syntax of the second approach makes it a bit easier for users to understand that they can support multiple brokers and means we don't have to write our own abstraction layer over the brokers. |
I actually tried to explore this using a sort of decorator pattern. Python uses an attribute like syntax to do this, but .net doesn't seem to support something like that. Except if we do reflection. Seemed to complicate the codebase more so I discarded that line of thought |
On another thought, we actually need the abstraction as it will be how we will update the published table and identify the broker that was used. |
I think I understand how the publishing works back. For the subscription aspect, an issue is to update the received table. Do we have an entry for every item that subscribes to the event? |
Apart from that, I can start implementing the publishing side of things. Can you list some tasks for me to do on this so I can ease into working on this. I have a few ideas I'll still prefer a little bit of handholding |
For publishing:
I'll be starting some of these today too. Edit: I'm on the fence about forcing a single database per handler as this will put us in a difficult situation when we support the concept of read models which are stores built based off events in Idea: maybe we introduce the concept of a Or, we can introduce |
Here is a good breakdown of the proper use of
TL:DR;
So, basically, because we have a one |
The only other thing I can think of that we should include is adding the For example, in the world of eCommerce, we could have a multi-step/service order creation process that creates an order, updates the product inventory, processes payment, and then ships the order. Through ack and nack we can choreograph the steps like so:
Through a series of For more complicated scenarios, we'll need to support more complex orchestration though the |
The The |
Here's how subscriptions are implemented in
|
Opting for transaction level abstraction over the use of
The
or, etc:
|
Update based on:
The text was updated successfully, but these errors were encountered: