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

Add context to all migrators #1183

Closed
ccoVeille opened this issue Nov 10, 2024 · 6 comments
Closed

Add context to all migrators #1183

ccoVeille opened this issue Nov 10, 2024 · 6 comments
Labels
Datasource enhancement New feature or request

Comments

@ccoVeille
Copy link
Contributor

As per discussed here

passing context is a better option for all other datasource migrators as our migrator interface doesn't have context as paramter in the following methods:

type migrator interface {
	checkAndCreateMigrationTable(c *container.Container) error
	getLastMigration(c *container.Container) int64

	beginTransaction(c *container.Container) transactionData

	commitMigration(c *container.Container, data transactionData) error
	rollback(c *container.Container, data transactionData)
}
@ccoVeille
Copy link
Contributor Author

But we have to wait for this one to be merged

@aryanmehrotra aryanmehrotra added enhancement New feature or request Datasource labels Nov 15, 2024
@afzal442
Copy link

Hi, I would like to take this up. This is going to be a straightforward upgrade. Right?

@afzal442
Copy link

cc @coolwednesday @aryanmehrotra any thought?

@nidhey27
Copy link
Contributor

nidhey27 commented Dec 2, 2024

@ccoVeille Can you quickly confirm the changes and clarify exactly what we are aiming to achieve?

@coolwednesday
Copy link
Contributor

coolwednesday commented Dec 2, 2024

Hey @nidhey27, @ccoVeille, @afzal442 ,

Thanks for the suggestion! After thinking it over, we’ve decided not to add this support right now. It might break things for folks already using migrations, and passing the context doesn’t seem to add much value in this case.

If there’s something we’re missing or a specific use case you had in mind, let us know—we’d be happy to revisit it!

@ccoVeille
Copy link
Contributor Author

OK.

@ccoVeille ccoVeille closed this as not planned Won't fix, can't repro, duplicate, stale Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datasource enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants