Some refactors regarding Logs & context.Context#7
Some refactors regarding Logs & context.Context#7jbret89 wants to merge 8 commits intoitimofeev:masterfrom
context.Context#7Conversation
|
Hello! |
| You can read more details about this pattern here https://microservices.io/patterns/data/saga.html#example-choreography-based-saga | ||
|
|
||
| # Installing | ||
| ```go get github.com/itimofeev/go-saga``` |
There was a problem hiding this comment.
I think that we should keep previous import in order to merge this MR. It'll be strange, if repository will be itimofeev, but module name will contain another name.
| Name string | ||
| Type string | ||
| Time time.Time | ||
| ExecutionID string `gorm:"index;type:varchar(255)"` |
There was a problem hiding this comment.
I have some doubts for keeping gorm tags in general Log struct, because not all users use gorm for storing logs.
What do you think if move them to gorm package and convert general Log struct to gorm one somewhere in gorm related code?
| logStore Store | ||
| } | ||
|
|
||
| func NewCoordinator(ctx context.Context, saga *Saga, logStore Store, executionID ...string) *ExecutionCoordinator { |
There was a problem hiding this comment.
While I like the idea to have ctx in methods that work with store I'm not sure that removing one of context is the good idea. First context was used to process direct flow when everything happens as expected. But what if user cancels the request, in that case we need to rollback some steps but we can't use the same already canceled context, because we will not be able to commit any rollback transactions.
Could you please clarify how your solution will work in that case?
2 participants
This PR aims to refactor some points showcased following: