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

refactor: separate sql stores from datasources #417

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Aug 7, 2024

What this PR changes/adds

Separate datasource from sql store configurations

Why it does that

Briefly state why the change was necessary.

Further notes

List other areas of code that have changed but are not necessarily linked to the main feature. This could be method
signature changes, package declarations, bugs that were encountered and were fixed inline, etc.

Linked Issue(s)

Closes #416

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

@ndr-brt ndr-brt added the refactoring Changing directories, modules, structures, etc. without functional changes label Aug 7, 2024
@@ -66,6 +71,7 @@
}

private String getDataSourceName(ServiceExtensionContext context) {
return context.getConfig().getString(DATASOURCE_SETTING_NAME, DataSourceRegistry.DEFAULT_DATASOURCE);
return DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
DataSourceName.getDataSourceName
should be avoided because it has been deprecated.
@@ -66,6 +70,6 @@
}

private String getDataSourceName(ServiceExtensionContext context) {
return context.getConfig().getString(DATASOURCE_SETTING_NAME, DataSourceRegistry.DEFAULT_DATASOURCE);
return DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
DataSourceName.getDataSourceName
should be avoided because it has been deprecated.
@@ -76,6 +80,6 @@
}

private String getDataSourceName(ServiceExtensionContext context) {
return context.getConfig().getString(DATASOURCE_SETTING_NAME, DataSourceRegistry.DEFAULT_DATASOURCE);
return DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
DataSourceName.getDataSourceName
should be avoided because it has been deprecated.
@@ -66,6 +70,6 @@
}

private String getDataSourceName(ServiceExtensionContext context) {
return context.getConfig().getString(DATASOURCE_SETTING_NAME, DataSourceRegistry.DEFAULT_DATASOURCE);
return DataSourceName.getDataSourceName(DATASOURCE_NAME, DATASOURCE_SETTING_NAME, context.getConfig(), context.getMonitor());

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
DataSourceName.getDataSourceName
should be avoided because it has been deprecated.
@ndr-brt ndr-brt force-pushed the 416-separate-stores-from-datasources branch from 1374eac to 2e06344 Compare August 7, 2024 09:14
@Setting(value = "Datasource name for the DidResource database", defaultValue = DataSourceRegistry.DEFAULT_DATASOURCE)
public static final String DATASOURCE_SETTING_NAME = "edc.datasource.credentials.name";
@Setting(value = "The datasource to be used", defaultValue = DataSourceRegistry.DEFAULT_DATASOURCE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i would call the deprecated property DATASOURCE_SETTING_NAME_DEPRECATED, and the new one DATASOURCE_SETTING_NAME

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately I didn't do that in other repositories so I this point I would just let it like this, anyway I think that the @Deprecated annotation should be enough

@ndr-brt ndr-brt merged commit 9e5da23 into main Aug 8, 2024
21 checks passed
@ndr-brt ndr-brt deleted the 416-separate-stores-from-datasources branch August 8, 2024 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Changing directories, modules, structures, etc. without functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate datasource and store configurations
3 participants