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

[9.x] TransactionCommitting #44608

Merged
merged 1 commit into from
Oct 19, 2022
Merged

[9.x] TransactionCommitting #44608

merged 1 commit into from
Oct 19, 2022

Conversation

rez1dent3
Copy link
Contributor

@rez1dent3 rez1dent3 commented Oct 15, 2022

Hello.

There was a need for a committing event, it would be great to add to the framework.

Thanks.

@ankurk91
Copy link
Contributor

Naming can be improved to align with other similar events

@rez1dent3 rez1dent3 changed the title [9.x] TransactionPreCommitted [9.x] TransactionCommitting (TransactionPreCommitted) Oct 15, 2022
@rez1dent3 rez1dent3 changed the title [9.x] TransactionCommitting (TransactionPreCommitted) [9.x] TransactionCommitting Oct 16, 2022
@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

@rez1dent3
Copy link
Contributor Author

@taylorotwell Hello. The event must be done on the side of the framework. A standalone package would be difficult to maintain and keep ManagesTransactions up to date. Any change to the framework will break the package. I believe adding an event won't harm your product. For example, this event can be used in logging, in profiling tools, etc.

For example, event sourcing model. Inside the transaction, we will fill some of our state and, before the last commit, write logs with one big insert request, inside the same transaction. These are all examples, but they are very important, I think.

@rez1dent3
Copy link
Contributor Author

@taylorotwell I will describe in detail my case of using this functionality. I am developing a package for working with virtual wallets. Now I have a custom service for working with transactions in the database. When developers call DB::beginTransaction() services in their code, I throw an exception. Package users ask to remove this exception (for example, they have problems with laravel nova bavix/laravel-wallet#455 bavix/laravel-wallet#569 bavix/laravel-wallet#570). I can't use native laravel transactions right now because before DB::commit(), I need to collect all requests and execute them in batches (to do this, I need to subscribe to the event before commit). To be sure of the atomicity of the written data, in case of an error, all data will be rolled back. Adding this event will help me a lot, without it there is no point in continuing to move away from our own transactions towards standardization. I understand that this is a rather narrow case. But I think that the event will not be superfluous in the framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants