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

Introducing primary data from SQL #98

Merged
merged 19 commits into from
Mar 22, 2022
Merged

Introducing primary data from SQL #98

merged 19 commits into from
Mar 22, 2022

Conversation

sebastian-peter
Copy link
Member

@sebastian-peter sebastian-peter commented Jan 18, 2022 •
edited
Loading

@sebastian-peter sebastian-peter self-assigned this Jan 18, 2022
@sebastian-peter sebastian-peter added the enhancement New feature or request label Jan 18, 2022
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@codecov
Copy link

codecov bot commented Jan 20, 2022 •
edited
Loading

Codecov Report

Merging #98 (dbc797d) into dev (f2820b2) will increase coverage by 0.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev      #98      +/-   ##
==========================================
+ Coverage   79.23%   79.36%   +0.13%     
==========================================
  Files         155      155              
  Lines        5726     5734       +8     
  Branches       79       79              
==========================================
+ Hits         4537     4551      +14     
+ Misses       1189     1183       -6
Impacted Files Coverage Δ
...3/simona/service/primary/PrimaryServiceProxy.scala 89.42% <0.00%> (-0.11%) ⬇️
.../simona/service/primary/PrimaryServiceWorker.scala 100.00% <0.00%> (ø)
...ain/scala/edu/ie3/simona/config/SimonaConfig.scala 60.70% <0.00%> (+0.09%) ⬆️
...cala/edu/ie3/simona/agent/grid/DBFSAlgorithm.scala 84.95% <0.00%> (+0.88%) ⬆️
...rc/main/scala/edu/ie3/simona/util/ConfigUtil.scala 57.62% <0.00%> (+0.96%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b7650b...dbc797d. Read the comment docs.

@sebastian-peter sebastian-peter marked this pull request as ready for review January 20, 2022 15:51
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
t-ober
t-ober requested changes Jan 21, 2022
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.

All in all looks good to me besides the minor stuff.

src/main/scala/edu/ie3/simona/service/primary/PrimaryServiceWorker.scala Show resolved Hide resolved
src/main/scala/edu/ie3/simona/config/SimonaConfig.scala Show resolved Hide resolved
src/test/scala/edu/ie3/simona/service/primary/PrimaryServiceWorkerSqlIT.scala Outdated Show resolved Hide resolved
src/test/scala/edu/ie3/simona/service/primary/PrimaryServiceWorkerSqlIT.scala Outdated Show resolved Hide resolved
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
ckittl
ckittl suggested changes Jan 27, 2022
View reviewed changes
Copy link
Member

@ckittl ckittl left a comment

Choose a reason for hiding this comment

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

To a very good extend, LGTM. However, there is a major design flaw with the fixed / configurable table name. For weather it is okay to be configurable, but in all other cases, especially to receive time series, it's not.

CHANGELOG.md Show resolved Hide resolved
src/main/resources/config/config-template.conf Outdated Show resolved Hide resolved
src/main/resources/config/config-template.conf Show resolved Hide resolved
...ces/edu/ie3/simona/service/primary/timeseries/its_p_9185b8c1-86ba-4a16-8dea-5ac898e8caa5.sql Outdated Show resolved Hide resolved
src/main/scala/edu/ie3/simona/service/primary/PrimaryServiceWorker.scala Outdated Show resolved Hide resolved
src/test/scala/edu/ie3/simona/service/primary/PrimaryServiceWorkerSqlIT.scala Outdated Show resolved Hide resolved
src/test/scala/edu/ie3/simona/service/primary/PrimaryServiceWorkerSqlIT.scala Show resolved Hide resolved
src/test/scala/edu/ie3/simona/service/primary/PrimaryServiceWorkerSqlIT.scala Outdated Show resolved Hide resolved
src/test/scala/edu/ie3/simona/service/primary/PrimaryServiceWorkerSqlIT.scala Outdated Show resolved Hide resolved
src/main/scala/edu/ie3/simona/config/SimonaConfig.scala Show resolved Hide resolved
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sebastian-peter sebastian-peter added the blocked externally Progress blocked by outside resource label Feb 15, 2022
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sebastian-peter sebastian-peter removed the blocked externally Progress blocked by outside resource label Feb 23, 2022
@sebastian-peter sebastian-peter mentioned this pull request Feb 23, 2022
Closed
@sebastian-peter sebastian-peter added the blocked externally Progress blocked by outside resource label Mar 1, 2022
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sebastian-peter sebastian-peter removed the blocked externally Progress blocked by outside resource label Mar 16, 2022
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sebastian-peter sebastian-peter marked this pull request as ready for review March 16, 2022 19:04
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sebastian-peter sebastian-peter mentioned this pull request Mar 16, 2022
Merged
@sebastian-peter
Copy link
Member Author

There's plans to re-introduce some interface methods in ie3-institute/PowerSystemDataModel#565. Maybe something to consider for reviews here

ckittl
ckittl previously approved these changes Mar 17, 2022
View reviewed changes
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@sonarqubegithubprchecks
Copy link

Passed

Analysis Details

0 Issues

  • Bug0 Bugs
  • Vulnerability0 Vulnerabilities
  • Code Smell0 Code Smells

Coverage and Duplications

  • 90 percent coverage92.59% Coverage (81.70% Estimated after merge)
  • 3 percent duplication0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: edu.ie3:simona

View in SonarQube

danielfeismann
danielfeismann approved these changes Mar 21, 2022
View reviewed changes
Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for your work!

@t-ober t-ober merged commit f709de3 into dev Mar 22, 2022
ckittl added a commit that referenced this pull request Mar 24, 2022
@ckittl
Merge remote-tracking branch 'origin/dev' into df/#98-transformer_sus…
4330597 
…ceptance
@sebastian-peter sebastian-peter deleted the sp/#34-sql-primary-data branch April 5, 2022 16:26
@sebastian-peter sebastian-peter added this to the Version 3.0 milestone Feb 17, 2023
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 approved these changes

@danielfeismann danielfeismann danielfeismann approved these changes

@johanneshiry johanneshiry Awaiting requested review from johanneshiry

@ckittl ckittl Awaiting requested review from ckittl

Assignees

@sebastian-peter sebastian-peter

Labels
enhancement New feature or request
Projects
None yet
Milestone
Version 3.0
Development

Successfully merging this pull request may close these issues.

Implement SQL input for primary data
4 participants
@sebastian-peter @ckittl @t-ober @danielfeismann