quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

Automatically detect new Flyway scripts in Dev Mode

Open knutwannheden opened this issue 2 years ago • 14 comments

Description

When running in dev mode it would be nice if new Flyway migration scripts (along side any existing migration scripts under src/main/resources/db/migration) would be detected and then executed when the user presses the Migrate button in the Flyway Dev UI.

Currently adding a new migration script and pressing Migrate has no effect. The application must be explicitly restarted (e.g. pressing s in the console).

Implementation ideas

No response

knutwannheden avatar Apr 29 '22 13:04 knutwannheden

/cc @cristhiank, @gastaldi, @geoand, @gsmet

quarkus-bot[bot] avatar Apr 29 '22 13:04 quarkus-bot[bot]

Can you check and see if https://github.com/quarkusio/quarkus/pull/25280 fixes this? I didn't test it

geoand avatar Apr 30 '22 09:04 geoand

@geoand I tested it and it partially works. Here a summary of the behavior with your PR:

  1. I can edit an existing migration script and using the UI actions clean and migrate work as expected. Although I am not entirely sure if a force restart of the application really is expected.
  2. Adding a new migration script doesn't work, because it wasn't added to the watch list when the application was started.
  3. Adding a new migration script and additionally also editing an existing script does work, because it force restarts the application, which in turn detects the new script.

AFAICT the hot deployment mechanism allows for GLOB patterns to be registered. This seems like it would be a good fit here. Although the processor would currently have to use a wildcard like *.sql, since the sql-migration-prefix is part of the runtime configuration and I assume that a recorder cannot emit HotDeploymentWatchedFileBuildItem items. To me it seems like this config property should be moved to the build-time config, but I don't see any problem with using a *.sql GLOB pattern.

Note: Flyway locations can also contain wildcards (even Ant-style ** wildcards): https://flywaydb.org/blog/organising-your-migrations. AFAICT the Java FileSystems "glob" PathMatcher also supports this, so I suppose even that should work out of the box.

knutwannheden avatar May 02 '22 08:05 knutwannheden

I think I should also mention that when attempting to delete an existing migration script and then running Clean and Migrate I got an NPE from the QuarkusClassLoader:

java.lang.NullPointerException: Cannot invoke "io.quarkus.bootstrap.classloading.ClassPathResource.getUrl()" because "res" is null
	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.getResources(QuarkusClassLoader.java:251)
	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.getResources(QuarkusClassLoader.java:198)
	at org.flywaydb.core.internal.resource.classpath.ClassPathResource.read(ClassPathResource.java:105)
	at org.flywaydb.core.internal.resolver.ChecksumCalculator.calculateChecksumForResource(ChecksumCalculator.java:64)
	at org.flywaydb.core.internal.resolver.ChecksumCalculator.calculate(ChecksumCalculator.java:43)
	at org.flywaydb.core.internal.resolver.sql.SqlMigrationResolver.getChecksumForLoadableResource(SqlMigrationResolver.java:126)
	at org.flywaydb.core.internal.resolver.sql.SqlMigrationResolver.addMigrations(SqlMigrationResolver.java:168)
	at org.flywaydb.core.internal.resolver.sql.SqlMigrationResolver.resolveMigrations(SqlMigrationResolver.java:71)
	at org.flywaydb.core.internal.resolver.sql.SqlMigrationResolver.resolveMigrations(SqlMigrationResolver.java:50)
	at org.flywaydb.core.internal.resolver.CompositeMigrationResolver.collectMigrations(CompositeMigrationResolver.java:107)
	at org.flywaydb.core.internal.resolver.CompositeMigrationResolver.doFindAvailableMigrations(CompositeMigrationResolver.java:90)
	at org.flywaydb.core.internal.resolver.CompositeMigrationResolver.resolveMigrations(CompositeMigrationResolver.java:83)
	at org.flywaydb.core.internal.resolver.CompositeMigrationResolver.resolveMigrations(CompositeMigrationResolver.java:41)
	at org.flywaydb.core.internal.info.MigrationInfoServiceImpl.refresh(MigrationInfoServiceImpl.java:92)
	at org.flywaydb.core.internal.command.DbMigrate.migrateGroup(DbMigrate.java:173)
	at org.flywaydb.core.internal.command.DbMigrate.lambda$migrateAll$0(DbMigrate.java:141)
	at org.flywaydb.core.internal.jdbc.TableLockingExecutionTemplate$1.call(TableLockingExecutionTemplate.java:36)
	at org.flywaydb.core.internal.jdbc.TransactionalExecutionTemplate.execute(TransactionalExecutionTemplate.java:55)
	at org.flywaydb.core.internal.jdbc.TableLockingExecutionTemplate.execute(TableLockingExecutionTemplate.java:31)
	at org.flywaydb.core.internal.database.base.Connection.lock(Connection.java:102)
	at org.flywaydb.core.internal.schemahistory.JdbcTableSchemaHistory.lock(JdbcTableSchemaHistory.java:139)
	at org.flywaydb.core.internal.command.DbMigrate.migrateAll(DbMigrate.java:141)
	at org.flywaydb.core.internal.command.DbMigrate.migrate(DbMigrate.java:98)
	at org.flywaydb.core.Flyway$1.execute(Flyway.java:172)
	at org.flywaydb.core.Flyway$1.execute(Flyway.java:124)
	at org.flywaydb.core.FlywayExecutor.execute(FlywayExecutor.java:205)
	at org.flywaydb.core.Flyway.migrate(Flyway.java:124)
	at io.quarkus.flyway.runtime.devconsole.FlywayDevConsoleRecorder$1.handlePost(FlywayDevConsoleRecorder.java:34)
	at io.quarkus.devconsole.runtime.spi.DevConsolePostHandler.dispatch(DevConsolePostHandler.java:47)
	at io.quarkus.devconsole.runtime.spi.DevConsolePostHandler$1.handle(DevConsolePostHandler.java:39)
	at io.quarkus.devconsole.runtime.spi.DevConsolePostHandler$1.handle(DevConsolePostHandler.java:36)
	at io.vertx.core.impl.AbstractContext.dispatch(AbstractContext.java:100)
	at io.vertx.core.impl.AbstractContext.dispatch(AbstractContext.java:63)
	at io.vertx.core.http.impl.HttpEventHandler.handleEnd(HttpEventHandler.java:76)
	at io.vertx.core.http.impl.Http1xServerRequest.onEnd(Http1xServerRequest.java:561)
	at io.vertx.core.http.impl.Http1xServerRequest.lambda$pendingQueue$1(Http1xServerRequest.java:126)
	at io.vertx.core.streams.impl.InboundBuffer.handleEvent(InboundBuffer.java:240)
	at io.vertx.core.streams.impl.InboundBuffer.drain(InboundBuffer.java:227)
	at io.vertx.core.streams.impl.InboundBuffer.lambda$fetch$0(InboundBuffer.java:280)
	at io.vertx.core.impl.AbstractContext.dispatch(AbstractContext.java:100)
	at io.vertx.core.impl.AbstractContext.dispatch(AbstractContext.java:63)
	at io.vertx.core.impl.EventLoopContext.lambda$runOnContext$0(EventLoopContext.java:38)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:164)
	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:469)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:503)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:986)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:833)

AFAICT this should not happen, as ClassLoader#getResources(String) is expected to return an empty Enumeration. As this PR hasn't been integrated yet, I can't really create an issue with a reproducer, but I will create an issue anyway.

Reported as issue #25294.

knutwannheden avatar May 02 '22 08:05 knutwannheden

AFAICT the hot deployment mechanism allows for GLOB patterns to be registered

Do you have examples of this? A quick scan of the code doesn't seem to indicate it would work...

geoand avatar May 03 '22 05:05 geoand

Do you have examples of this? A quick scan of the code doesn't seem to indicate it would work...

I couldn't find any examples, but I found the RuntimeUpdatesProcessor#expandGlobPattern() method which sure looks promising: https://github.com/quarkusio/quarkus/blob/1f900a8e42d9131f10846c3cb9c6a7be09484b37/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesProcessor.java#L1170-L1192

See also #16005

knutwannheden avatar May 03 '22 06:05 knutwannheden

Interesting, I didn't know that.

Feel free to try it out, since I won't have time for this probably until next week

geoand avatar May 03 '22 07:05 geoand

@geoand OK. I will look into providing a PR.

Question: Should I create an issue / PR to change some config properties from runtime to build-time? I think it would make sense, but strictly speaking this would then be a breaking change.

knutwannheden avatar May 03 '22 07:05 knutwannheden

Should I create an issue / PR to change some config properties from runtime to build-time? I think it would make sense, but strictly speaking this would then be a breaking change.

Yeah, that would be something different and definitely a breaking change. No need for an issue, just a PR explaining why it makes sense.

geoand avatar May 03 '22 08:05 geoand

I get the impression that the Quarkus application isn't being restarted when a SQL migration file is being deleted, even though the file was registered as requiring a restart. I need to do some more digging to verify this.

knutwannheden avatar May 03 '22 10:05 knutwannheden

@geoand The glob pattern matching only partially works: On Windows it doesn't appear to work at all because InvalidPathExceptions get thrown whenever a Path is constructed using a string containing a wildcard and even when I fix that, there are more problems preventing this from working. Finally, the pattern matching is only applied when setting up the files to be watched, not when checking for changes (i.e. checking if there are any new files matching any of the watched patterns), so for the use case of detecting new migration scripts, this mechanism is not usable in its current form. I will create a separate issue for that.

That being said, I think integrating #25280 would be a step forward, as it allows to modify existing migration scripts while running in dev mode. So I would suggest we integrate that fix and keep this issue open (or create a new issue for the remaining open points).

knutwannheden avatar May 03 '22 14:05 knutwannheden

As noted here, we currently face issues with filesystem:-prefixed locations that I think should be back with https://github.com/quarkusio/quarkus/pull/26168.

mzellho avatar Jun 15 '22 20:06 mzellho

Not sure if related (I'm a novice with Quarkus) but I am trying to run a Quarkus app in a local container and use remote development mode and I'm facing the same error:

backend     | 2022-09-20 14:56:35,633 ERROR [io.qua.dep.dev.IsolatedDevModeMain] (vert.x-worker-thread-1) Failed to start quarkus: java.lang.RuntimeException: java.lang.NullPointerException: Cannot invoke "io.quarkus.bootstrap.classloading.ClassPathResource.getUrl()" because "res" is null
backend     | 	at io.quarkus.runner.bootstrap.AugmentActionImpl.runAugment(AugmentActionImpl.java:330)
backend     | 	at io.quarkus.runner.bootstrap.AugmentActionImpl.reloadExistingApplication(AugmentActionImpl.java:265)
backend     | 	at io.quarkus.runner.bootstrap.AugmentActionImpl.reloadExistingApplication(AugmentActionImpl.java:60)
backend     | 	at io.quarkus.deployment.dev.IsolatedDevModeMain.restartApp(IsolatedDevModeMain.java:251)
backend     | 	at io.quarkus.deployment.dev.IsolatedDevModeMain.restartCallback(IsolatedDevModeMain.java:234)
backend     | 	at io.quarkus.deployment.dev.RuntimeUpdatesProcessor.doScan(RuntimeUpdatesProcessor.java:536)
backend     | 	at io.quarkus.deployment.dev.RuntimeUpdatesProcessor.doScan(RuntimeUpdatesProcessor.java:436)
backend     | 	at io.quarkus.vertx.http.runtime.devmode.VertxHttpHotReplacementSetup$4.handle(VertxHttpHotReplacementSetup.java:152)
backend     | 	at io.quarkus.vertx.http.runtime.devmode.VertxHttpHotReplacementSetup$4.handle(VertxHttpHotReplacementSetup.java:139)
backend     | 	at io.vertx.core.impl.ContextBase.lambda$null$0(ContextBase.java:137)
backend     | 	at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:264)
backend     | 	at io.vertx.core.impl.ContextBase.lambda$executeBlocking$1(ContextBase.java:135)
backend     | 	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
backend     | 	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2449)
backend     | 	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1452)
backend     | 	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
backend     | 	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
backend     | 	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
backend     | 	at java.base/java.lang.Thread.run(Thread.java:833)
backend     | Caused by: java.lang.NullPointerException: Cannot invoke "io.quarkus.bootstrap.classloading.ClassPathResource.getUrl()" because "res" is null
backend     | 	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.getResources(QuarkusClassLoader.java:250)
backend     | 	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.getResources(QuarkusClassLoader.java:265)
backend     | 	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.getResources(QuarkusClassLoader.java:197)
backend     | 	at io.smallrye.common.classloader.ClassPathUtils.consumeAsPaths(ClassPathUtils.java:84)
backend     | 	at io.smallrye.config.AbstractLocationConfigSourceLoader.tryClassPath(AbstractLocationConfigSourceLoader.java:128)
backend     | 	at io.smallrye.config.AbstractLocationConfigSourceLoader.loadConfigSources(AbstractLocationConfigSourceLoader.java:93)
backend     | 	at io.smallrye.config.AbstractLocationConfigSourceLoader.loadConfigSources(AbstractLocationConfigSourceLoader.java:76)
backend     | 	at io.quarkus.runtime.configuration.ApplicationPropertiesConfigSourceLoader$InClassPath.getConfigSources(ApplicationPropertiesConfigSourceLoader.java:30)
backend     | 	at io.quarkus.runtime.configuration.ApplicationPropertiesConfigSourceLoader$InClassPath.getConfigSources(ApplicationPropertiesConfigSourceLoader.java:27)
backend     | 	at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:439)
backend     | 	at io.quarkus.deployment.ExtensionLoader.loadStepsFrom(ExtensionLoader.java:179)
backend     | 	at io.quarkus.deployment.QuarkusAugmentor.run(QuarkusAugmentor.java:105)
backend     | 	at io.quarkus.runner.bootstrap.AugmentActionImpl.runAugment(AugmentActionImpl.java:328)
backend     | 	... 18 more

I am not using any database migration tool. Could it be related?

y-luis-rojo avatar Sep 20 '22 15:09 y-luis-rojo

@y-luis it seems like a bug. I created https://github.com/quarkusio/quarkus/issues/28098 to verify that

gastaldi avatar Sep 20 '22 16:09 gastaldi