vapor / fluent-kit

Swift ORM (queries, models, and relations) for NoSQL and SQL databases
MIT License
211 stars 116 forks source link

Release 1.48.0 successfully blocks execution of method `try app.autoMigrate().wait()` #602

Closed mkll closed 5 months ago

mkll commented 5 months ago

Describe the issue

Release 1.48.0 successfully blocks execution of method try app.autoMigrate().wait()

Vapor version

4.94.0

Operating system and version

Debian 12

Swift version

Swift Package Manager - Swift 5.9.2

Steps to reproduce

  1. Update fluent kit to version 1.48.0
  2. We've arrived

Outcome

Vapor execution stops at .wait()

Additional notes

No response

gwynne commented 5 months ago

If you're using .wait() with app.autoMigrate(), you're most likely using an outdated version of the app entrypoint. The configure() method is async by default in the current app template, allowing you to write try await app.autoMigrate(). The pregenerated PostgreSQL template - in particular the entrypoint.swift and configure.swift files - provides an example starting point. In general, except in very unusual cases, there should no longer be any need to use .wait() for anything. Even if there is no async version of a futures-based method, you can use the EventLoopFuture.get() helper to turn any future into an await-able expression.

This having been said, none of the changes made in 1.48.0 should have affected this. My guess would be that you were lucking into it working before and that the added Sendable annotations were enough to throw that off.

mkll commented 5 months ago

@gwynne, My use of the legacy entry point is because I (surprise!) do not use Swift Structured Concurrency. My project did not start yesterday, it has been going on for more than three years, in the pre-async/await era, and it's relatively large, so it's hard to rewrite quickly.

In the same time, the SemVer principles tell us that without changing the major version, the API should not break. We see it breaking down, little by little, piece by piece, component by component. By updating dependencies today, I never know which one will break tomorrow. The major version remains the same.

We are gradually losing the ability to use the legacy API based on EventLoopFutures without any official announcement that it is no longer supported. But its support breaks down here and there, spontaneously.

This is not a complaint (although I am of course saddened by this state of affairs), it is simply a statement of the fact that, while formally remaining within the major version, Vapor is no longer quite version 4. This, in turn, means that upToNextMajor no longer works, and we should abandon it.

So, I'd like to know in advance if it's time to freeze the Vapor dependency versions as well as the version of itself, or if I can wait a little longer on this.

Thanks!

P.S. Since the situation is fully resolved, it would be better to close this issue.

gwynne commented 5 months ago

You are correct - if the EventLoopFuture-based API was broken by these changes, that is a real bug and needs to be addressed.

gwynne commented 5 months ago

Can you see if #603 (and, ideally, vapor/fluent#774) fixes this issue? Easiest way to test is to temporarily add these lines to the dependencies for your app target in your Package.swift:

.package(url: "https://github.com/vapor/fluent-kit.git", branch: "oldstyle-migration-fix-maybe"),
.package(url: "https://github.com/vapor/fluent.git", branch: "sendable-fixup"),

If it doesn't work with both, try it with only the FluentKit override. (And don't forget to revert the change later!)

mkll commented 5 months ago

@gwynne Yes it helped (with both)!

gwynne commented 5 months ago

Awesome, thanks for bearing with me on this! I'll make sure both PRs get merged and releases tagged ASAP.

mkll commented 5 months ago

Wonderful!

gwynne commented 5 months ago

FluentKit 1.48.1 and Fluent 4.10.0 have been released, let me know if you run into any more issues!

mkll commented 5 months ago

@gwynne I have already managed to rebuild the project with the new release and everything works. Thanks!