Conversation
96d9921 to
19b5de0
Compare
| @Override | ||
| public void start() throws Exception { | ||
| initializeDataSource(); | ||
| } |
There was a problem hiding this comment.
Trenne bitte die Config von dem Managed, so dass initializeDataSource gekapselt ist und nur einmal pro Connection aufgerufen werden kann
| @JsonIgnore | ||
| private HikariDataSource dataSource; | ||
|
|
||
| @JsonIgnore | ||
| private HealthCheckRegistry healthCheckRegistry; |
There was a problem hiding this comment.
State bitte in einem Service halten, nicht in der Config.
| log.info("SUCCESS connecting to {}", getJdbcConnectionUrl()); | ||
| } | ||
| else { | ||
| log.error("FAILED connecting to {}. Connection did not become valid.", getJdbcConnectionUrl()); |
There was a problem hiding this comment.
Sind in der Url Credentials enthalten?
| IdColumnConfig idColumns = config.getIdColumns(); | ||
| SqlConnectorConfig sqlConnectorConfig = config.getSqlConnectorConfig(); | ||
| DatabaseConfig databaseConfig = sqlConnectorConfig.getDatabaseConfig(namespaceStorage.getDataset()); | ||
| DatabaseConnection databaseConfig = sqlConnectorConfig.getDatabaseConfig(namespaceStorage.getDataset()); |
There was a problem hiding this comment.
| DatabaseConnection databaseConfig = sqlConnectorConfig.getDatabaseConfig(namespaceStorage.getDataset()); | |
| DatabaseConnection databaseConnection = sqlConnectorConfig.getDatabaseConfig(namespaceStorage.getDataset()); |
| initializeDataSource(); | ||
| } | ||
|
|
||
| public void initializeDataSource() { |
There was a problem hiding this comment.
Zum Beispiel so
| public void initializeDataSource() { | |
| public ManagedDataSource initializeDataSource() { |
| * Keys must match the name of existing {@link Dataset}s. | ||
| */ | ||
| private Map<String, @Valid DatabaseConfig> databaseConfigs; | ||
| private Map<DatasetId, @Valid DatabaseConnection> databaseConfigs; |
There was a problem hiding this comment.
Funktioniert die Deserialisierung der Keys einfach so ohne Annotationen?
| public void close() { | ||
| closeDslContextWrapper(); | ||
| //TODO do we even need to shutdown the connection? | ||
| super.close(); | ||
| } |
There was a problem hiding this comment.
Denke schon, wenn man ein Dataset löscht und dann wieder anlegt, sollte sich auch die Connection erneuern.
There was a problem hiding this comment.
Das Ding ist halt, die Connection liegt nicht im Dataset sondern in der config.json
There was a problem hiding this comment.
Deswegen sollte das nicht in der Config gehalten werden, sondern in einem separaten Objekt gemanaged werden. Bei Solr habe ich letztendlich die Dataset bezogenen Connections nicht über den Lifecycle gemanaged, weil sie nur bedingt an diesen gebunden sind.
| getDialect().getJooqDialect(), | ||
| settings | ||
| ); | ||
| } |
There was a problem hiding this comment.
Bitte diese Methode auch in eine ManagedDatasource packen
| // TODO this could be implemented as a plugin tbh | ||
| if(config.getSqlConnectorConfig() != null) { | ||
| config.getSqlConnectorConfig().initialize(environment); | ||
| } |
There was a problem hiding this comment.
Oder ein ConfiguredBundle, da kannst du checken ob SqlConnectorConfig gesetzt/enabled ist, Resources/Services registrieren für Injection, Healthchecks setzten...
No description provided.