DefaultTaskConfigurer constructors should use TaskProperties.getTablePrefix() instead of constant TaskProperties.DEFAULT_TABLE_PREFIX

[nightswimmings]

trafficstars

Right now if one uses DefaultTaskConfigurer without specifying a prefix, it looses the ability to use the one from properties. I think when not passed as a param, it should use TaskProperties.getTablePrefix() and instead set the default at the TaskProperties level

Besides, shouldn't "Class.forName("javax.persistence.EntityManager");" be now "Class.forName("jakarta.persistence.EntityManager");" considering this corresponds to the release train for Boot 3.2?

[cppwfs]

Thank you for the comment and good catch on the javax.
But, I'm a little confused, as to why we'd want to pass in the TaskProperties to obtain the prefix when other constructors allow for the setting of the prefix?

[nightswimmings]

Because if we want to customize only one argument then we don't have to deal with and research internal mechanisms for table prefix I mean I think it makes sense to assume that as long as you don't pass the tablePrefix as argument, you want to benefit from the default properties mechanism

Think somebody that just wants to pass a custom datasource, then he performs

public CustomTaskConfigurer(@BatchDataSource DataSource dataSource) {
        super(dataSource); 
}

But then he realises that the tables are being created in another place (when using latest SpringBoot3-enabled SCDF, for instance), he or she needs to research the implementation and understand that you need to reproduce the previous default

public CustomTaskConfigurer(@BatchDataSource DataSource dataSource, TaskProperties taskProperties) {
        super(dataSource, taskProperties.getTablePrefix(), null); 
    }

It's an undersirable side-effect, because you are extending DefaultTaskConfigurer, so everything yoo don't explicitly touch, should remain as the Default configuration according to the name

Indeed ideally IMO it should be something like

(@EnabledConfigurationProperties TaskProperties) ? taskProperties.getTablePrefix() : DEFAULT_TABLE_PREFIX

[favna]

I'm facing this same issue now where we provide a custom DataSource and then I get this error:

Caused by: java.lang.IllegalArgumentException: Invalid TaskExecution, ID 12 not found

Our custom TaskConfigurer is:

@Component
@Profile("!springBootTest & !commandline")
class TaskConfigurer extends DefaultTaskConfigurer {
	public TaskConfigurer(DataSourceWrapper dataSourceWrapper) {
		super(dataSourceWrapper.getDataSource());
	}
}

and the relevant other classes are:

import com.zaxxer.hikari.HikariDataSource;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.jdbc.DataSourceProperties;
import org.springframework.context.annotation.Profile;
import org.springframework.stereotype.Component;

@Component
@lombok.Value
@Profile("!springBootTest & !commandline")
class DataSourceWrapper implements AutoCloseable {
	HikariDataSource dataSource;

	DataSourceWrapper(@Value("${spring.datasource.hikari.maximum-pool-size:10}") int maxPoolSize,
	                  @Value("${spring.datasource.hikari.minimum-idle:1}") int minimumIdle,
	                  @Value("${spring.datasource.hikari.idle-timeout:30000}") int idleTimeout,
	                  @Qualifier("dataflowDataSourceProperties") DataSourceProperties dataSourceProperties) {
		var hikariDataSource = dataSourceProperties.initializeDataSourceBuilder()
				.type(HikariDataSource.class)
				.build();

		hikariDataSource.setMaximumPoolSize(maxPoolSize);
		hikariDataSource.setMinimumIdle(minimumIdle);
		hikariDataSource.setMinimumIdle(idleTimeout);

		this.dataSource = hikariDataSource;
	}

	@Override
	public void close() {
		this.dataSource.close();
	}
}

import org.springframework.boot.autoconfigure.jdbc.DataSourceProperties;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;

@Configuration
@Profile("!springBootTest")
@ConfigurationProperties("spring.datasource")
public class DataSourceConfig {
	@Bean
	public DataSourceProperties dataflowDataSourceProperties() {
		return new DataSourceProperties();
	}
}

I am unsure if there is a way to work around this issue at the moment, but if not then it is a major blocker for our upgrades to using boot3 mode in SCDF 2.11.2. The only constructor for DefaultTaskConfigurer that I can see that allows overriding both DataSource and tablePrefix is

public DefaultTaskConfigurer(DataSource dataSource, String tablePrefix, ApplicationContext context)

but then I would have no idea what to provide for ApplicationContext nor if I can even reliable get the desired tablePrefix in my custom TaskConfigurer.

We use Pivotal CloudFoundry and in the User provided environment variables I do see that a boot3 related tablePrefix gets assigned:

[cppwfs]

Let's see If we can squeeze this into 3.1.2

[cppwfs]

Since this is a change in behavior this will be in the 3.2 release of Task.

[favna]

Can you provide any rough estimation whatsoever for when that may be released so I can update our Jira with more information? For now, the story is flagged in the current sprint (2 weeks remaining for it) but if 3.2 won't be coming for some time I'd rather push it back to the backlog.

[cppwfs]

I'm looking at giving the user the ability to pass in a TaskProperties into the constructor and then letting the DefaultTaskConfiguration constructor resolve tablePrefix as @nightswimmings suggested above. Since this solution doesn't affect the default behavior I can put it back onto the 3.1.2 release.

[cppwfs]

This has been pushed to 3.2, based on discussions that a change like this can't be put into a patch release.

[favna]

Thanks for the update. I'll wait patiently, no problem. Plenty of other things on our backlog that need doing.