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.

Sign up for GitHub

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

Jump to bottom

Implement SQL sinks #903

Open
wants to merge 27 commits into
base: dev
Choose a base branch
from
Open

Implement SQL sinks #903

wants to merge 27 commits into from

Conversation

jo-bao
Copy link
Contributor

@jo-bao jo-bao commented Oct 12, 2023 •
edited
Loading

Resolves #762

@jo-bao jo-bao closed this Oct 12, 2023
@jo-bao jo-bao deleted the jb/#762-sql-grid-datasources branch October 12, 2023 13:39
@jo-bao jo-bao restored the jb/#762-sql-grid-datasources branch October 12, 2023 13:39
@jo-bao jo-bao reopened this Oct 12, 2023
@jo-bao jo-bao marked this pull request as draft October 12, 2023 13:42
t-ober
t-ober requested changes Oct 16, 2023
View reviewed changes
Copy link
Contributor

@t-ober t-ober left a comment

Choose a reason for hiding this comment

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

That looks already very good. Very nice work @jo-bao 🚀

src/main/java/edu/ie3/datamodel/io/sink/SqlSink.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/sink/SqlSink.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/DatabaseIdentifier.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/DatabaseIdentifier.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/DatabaseIdentifier.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/sink/SqlSink.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/sink/SqlSink.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/sink/SqlSink.java Outdated Show resolved Hide resolved
src/test/groovy/edu/ie3/datamodel/io/sink/SqlSinkTest.groovy Outdated Show resolved Hide resolved
src/test/groovy/edu/ie3/datamodel/io/sink/SqlSinkTest.groovy Outdated Show resolved Hide resolved
@jo-bao jo-bao marked this pull request as ready for review November 22, 2023 08:37
@sebastian-peter
Copy link
Member

I guess this was blocked by the PSU not being released, but can now be unblocked by #1006

jo-bao and others added 5 commits February 26, 2024 09:34
@jo-bao
Merge remote-tracking branch 'origin/dev' into jb/#762-sql-grid-datas…
31da1fd 
…ources

# Conflicts:
#	src/test/groovy/edu/ie3/test/common/TimeSeriesTestData.groovy
@jo-bao
Merge branch 'dev' into jb/#762-sql-grid-datasources
4243e57 
# Conflicts:
#	src/main/java/edu/ie3/datamodel/io/naming/DatabaseNamingStrategy.java
fixing tests
d85e234
@jo-bao
replace UniqueEntity
15d97cc
@jo-bao
Main Method
d77e478
@jo-bao jo-bao requested review from sebastian-peter and removed request for sebastian-peter July 2, 2024 11:44
@jo-bao jo-bao added the io Issues relating to input/output label Jul 3, 2024
t-ober
t-ober requested changes Aug 9, 2024
View reviewed changes
Copy link
Contributor

@t-ober t-ober left a comment

Choose a reason for hiding this comment

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

Looks very good. Some minor things.
Also make sure to take another look at the Sonarqube complaints

src/test/resources/edu/ie3/datamodel/io/sink/_sql/setup.sql Show resolved Hide resolved
src/test/resources/edu/ie3/datamodel/io/sink/_sql/load_profile.sql Show resolved Hide resolved
src/test/groovy/edu/ie3/datamodel/io/sink/SqlSinkTest.groovy Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/DbGridMetadata.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/sink/SqlSink.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/sink/SqlSink.java Outdated Show resolved Hide resolved
src/main/java/edu/ie3/datamodel/io/sink/SqlSink.java
Comment on lines +288 to +357
RawGridElements rawGridElements = jointGridContainer.getRawGrid();
Set<NodeInput> nodes = rawGridElements.getNodes();
Set<LineInput> lines = rawGridElements.getLines();
Set<Transformer2WInput> transformer2Ws = rawGridElements.getTransformer2Ws();
Set<Transformer3WInput> transformer3Ws = rawGridElements.getTransformer3Ws();
Set<SwitchInput> switches = rawGridElements.getSwitches();
Set<MeasurementUnitInput> measurementUnits = rawGridElements.getMeasurementUnits();

// get system participants with types or operators
SystemParticipants systemParticipants = jointGridContainer.getSystemParticipants();
Set<BmInput> bmPlants = systemParticipants.getBmPlants();
Set<ChpInput> chpPlants = systemParticipants.getChpPlants();
Set<EvcsInput> evCS = systemParticipants.getEvcs();
Set<EvInput> evs = systemParticipants.getEvs();
Set<FixedFeedInInput> fixedFeedIns = systemParticipants.getFixedFeedIns();
Set<HpInput> heatPumps = systemParticipants.getHeatPumps();
Set<LoadInput> loads = systemParticipants.getLoads();
Set<PvInput> pvPlants = systemParticipants.getPvPlants();
Set<StorageInput> storages = systemParticipants.getStorages();
Set<WecInput> wecPlants = systemParticipants.getWecPlants();

// get graphic elements (just for better readability, we could also just get them directly
// below)
GraphicElements graphicElements = jointGridContainer.getGraphics();

// extract types
Set<AssetTypeInput> types =
Stream.of(
lines,
transformer2Ws,
transformer3Ws,
bmPlants,
chpPlants,
evs,
heatPumps,
storages,
wecPlants)
.flatMap(Collection::stream)
.map(Extractor::extractType)
.collect(Collectors.toSet());

// extract operators
Set<OperatorInput> operators =
Stream.of(
nodes,
lines,
transformer2Ws,
transformer3Ws,
switches,
measurementUnits,
bmPlants,
chpPlants,
evCS,
evs,
fixedFeedIns,
heatPumps,
loads,
pvPlants,
storages,
wecPlants)
.flatMap(Collection::stream)
.map(Extractor::extractOperator)
.flatMap(Optional::stream)
.collect(Collectors.toSet());

List<Entity> toAdd = new LinkedList<>();
toAdd.addAll(rawGridElements.allEntitiesAsList());
toAdd.addAll(systemParticipants.allEntitiesAsList());
toAdd.addAll(graphicElements.allEntitiesAsList());
toAdd.addAll(types);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you get all the elements with the e.g. systemParticipants.allEntitiesAsList() method ? The types and operators should be extracted automatically when persisting the participants, don't they?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it will be that easy, because not every system participant has a type. But maybe we can move this to another issue, because csv related it's the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

It does because you handle nested entities inside persistAll

  /** Persists a whole {@link JointGridContainer}. */
  public void persistJointGrid(JointGridContainer jointGridContainer, UUID gridUUID) {
    DbGridMetadata identifier = new DbGridMetadata(jointGridContainer.getGridName(), gridUUID);
    List<Entity> toAdd = new LinkedList<>();
    toAdd.addAll(jointGridContainer.allEntitiesAsList());
    persistAll(toAdd, identifier);
  }

is sufficient

Copy link
Contributor

Choose a reason for hiding this comment

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

Also please leave threads with open points / discussions unresolved, that makes it easier to refer back to 🙂

src/test/resources/edu/ie3/datamodel/io/sink/_sql/sql_test_data.sql Outdated Show resolved Hide resolved
t-ober
t-ober requested changes Aug 9, 2024
View reviewed changes
Copy link
Contributor

@t-ober t-ober left a comment

Choose a reason for hiding this comment

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

Looks very good. Some minor things.
Also make sure to take another look at the Sonarqube complaints

@jo-bao jo-bao requested a review from t-ober August 30, 2024 07:53
t-ober
t-ober requested changes Sep 25, 2024
View reviewed changes
Copy link
Contributor

@t-ober t-ober left a comment

Choose a reason for hiding this comment

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

Just this one open point 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-ober t-ober t-ober requested changes

@sebastian-peter sebastian-peter Awaiting requested review from sebastian-peter

Requested changes must be addressed to merge this pull request.

Assignees

@jo-bao jo-bao

Labels
io Issues relating to input/output
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adding SQL sink
3 participants
@jo-bao @sebastian-peter @t-ober