-
Notifications
You must be signed in to change notification settings - Fork 176
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: abstract data explorer modules from Influx-related code #2803
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @bossenti,
thanks for the PR.
I like the idea of the env variable.
I just have some minor questions, see comments.
Cheers,
Philipp
streampipes-commons/src/main/java/org/apache/streampipes/commons/constants/Envs.java
Outdated
Show resolved
Hide resolved
import org.apache.streampipes.dataexplorer.api.IDataExplorerManager; | ||
import org.apache.streampipes.dataexplorer.influx.DataExplorerManagerInflux; | ||
|
||
public enum DataExplorerDispatcher { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to change this to a class that we can more easily mock for testing instead of a singleton?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed DataExplorerDispatcher
to a regular class, check if that suits your expectations
Purpose
This PR continues the work of #2795 and abstracts the data explorer code from Influx-specific implementations. The idea is that whenever we want to interact with our data explorer storage we do not point to storage-specific implementations but use the
DataExplorerDispatcher
instead. TheDataExplorerDispatcher
gives aces to an an instance ofDataExplorerManager
which is tied to the underlying storage dependent on the configuration of StreamPipes. Therefore, this PR introduces the environment variableSP_TS_STORAGE
which determines the storage to be used and defaults toinfluxDB
.As for now, Influx is the only storage supported but this allows us to better separate our codebase and to integrate other time series storages as well.
Remarks
PR introduces (a) breaking change(s): yes
PR introduces (a) deprecation(s): no
Breaking Changes
This PR strongly moves files between different modules and therefore import paths might change, especially with respect to the module
streampipes-data-explorer-influx
. This PR does not have any affect on the behavior or any feature of StreamPipes.