Fix config auto-generation #131

ckittl · 2022-02-08T13:10:36Z

Fixes #130
Fixes #148

johanneshiry · 2022-02-08T14:49:27Z

CI is broken
I would prefer to know exactly where, that is in which PR this stuff has become broken. Doesn't feel well for me to approve & merge stuff where we assume that this might have been broken some time w/o knowing the source of this apparently invalid code! In particular not bc current dev is not broken (at least CI is passing?).

sebastian-peter

I just have some tscfg-related comments. The long comment could also be addressed in a separate PR.

Furthermore, genConfigSample currently does not work without specifying a destination directory for config classes (--dd). Looks like a bug in tscfg, I will investigate further. Edit: We might also think about simplifying the workflow and generating a config template each time a config class is generated.

gradle/scripts/tscfg.gradle

…eneration

codecov · 2022-02-17T15:31:16Z

Codecov Report

Merging #131 (97e551a) into dev (ac43abf) will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev     #131      +/-   ##
==========================================
+ Coverage   79.55%   79.62%   +0.06%     
==========================================
  Files         156      156              
  Lines        5757     5757              
  Branches       79       79              
==========================================
+ Hits         4580     4584       +4     
+ Misses       1177     1173       -4

Impacted Files	Coverage Δ
...e3/simona/agent/participant/ParticipantAgent.scala	`78.40% <0.00%> (-4.55%)`	⬇️
...n/scala/edu/ie3/simona/config/ConfigFailFast.scala	`90.18% <0.00%> (-0.50%)`	⬇️
...cala/edu/ie3/simona/config/ConfigConventions.scala	`100.00% <0.00%> (ø)`
...ain/scala/edu/ie3/simona/config/SimonaConfig.scala	`61.94% <0.00%> (+1.04%)`	⬆️
.../scala/edu/ie3/simona/config/RefSystemParser.scala	`100.00% <0.00%> (+2.85%)`	⬆️
...in/scala/edu/ie3/simona/config/VoltLvlParser.scala	`62.50% <0.00%> (+17.04%)`	⬆️

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 ac43abf...97e551a. Read the comment docs.

sebastian-peter · 2022-02-22T12:37:17Z

I still get a FileNotFoundException: \tmp\ExampleCfg.java when executing genConfigSample, is it just me?

If not, #148 should probably be fixed in a different PR.

johanneshiry

Thanks for your work. Looks 90% good to me. You may consider my suggestions with a slightly increased verbosity (verbosity has been high already, but I thought an additional hint might be helpful). Additionally, I have one questions regarding the deleted part of the experiments config as I couldn't find anything about it in the linked issues. You might explain the removal.

Thanks again :)

johanneshiry · 2022-02-22T20:50:47Z

gradle/scripts/tscfg.gradle

-task genExperimentsConfigClass {
-  doLast {
-    def tscfgJarFile = project.file('build/tscfg-' + tscfgVersion + '.jar')
-    if (!tscfgJarFile.exists() || !tscfgJarFile.isFile()) {
-      download {
-        src 'https://github.com/carueda/tscfg/releases/download/v' + tscfgVersion + '/tscfg-' + tscfgVersion + '.jar'
-        dest buildDir
-      }
-    }
-    javaexec {
-      main = "-jar"
-      args = [
-        "build/tscfg-${tscfgVersion}.jar",
-        "--spec",
-        "src/main/resources/config/experiments-config-template.conf",
-        "--scala",
-        "--durations",
-        "--pn",
-        "edu.ie3.simona.config",
-        "--cn",
-        "ExperimentsConfig",
-        "--dd",
-        "src/main/scala/edu/ie3/simona/config/"
-      ]
-    }
-  }
-}
-
-


Why? Does not relate to #130 nor #148

I just recognized in the discussion with sebastian that this is code, that was meant to be introduced with #66, but due to some error already is in code base right now.

src/main/scala/edu/ie3/simona/config/ConfigFailFast.scala

Co-authored-by: Johannes Hiry <johannes.hiry@tu-dortmund.de>

…eneration

ckittl · 2022-03-08T10:12:06Z

I still get a FileNotFoundException: \tmp\ExampleCfg.java when executing genConfigSample, is it just me?

If not, #148 should probably be fixed in a different PR.

According to the error message, you quoted, I assume you are running the command in windows os?

by choosing the OS dependent temporary file path

ckittl · 2022-03-08T12:42:57Z

I still get a FileNotFoundException: \tmp\ExampleCfg.java when executing genConfigSample, is it just me?
If not, #148 should probably be fixed in a different PR.

According to the error message, you quoted, I assume you are running the command in windows os?

Okay... It was an issue with the default destination directory in tscfg. I handed in a PR at carueda/tscfg#151, that might make this workaround obsolete.

ckittl · 2022-03-11T16:18:31Z

I still get a FileNotFoundException: \tmp\ExampleCfg.java when executing genConfigSample, is it just me?
If not, #148 should probably be fixed in a different PR.

According to the error message, you quoted, I assume you are running the command in windows os?

Okay... It was an issue with the default destination directory in tscfg. I handed in a PR at carueda/tscfg#151, that might make this workaround obsolete.

Merged, but not yet released.

sebastian-peter

With this workaround, genConfigSample works again, thank you. I created #178 to remove the workaround once it's not necessary anymore. So from my side, this is good to go.

t-ober

Looks good - thanks!

t-ober

Looks good codewise. But the Branch can't be built because of some issues in the PrimaryServiceProxy e.g. PrimaryServiceProxy.scala:151:16: value timeSeriesMetaInformation is not a member of edu.ie3.datamodel.io.source.csv.CsvTimeSeriesMappingSource

Please check that before merging. Although I'm not quite sure why this issue occurs since the class was not touched. I think some changes in main might not have been merged into this branch correctly

sonarqubegithubprchecks · 2022-04-11T20:21:01Z

Analysis Details

0 Issues

0 Bugs
0 Vulnerabilities
0 Code Smells

Coverage and Duplications

100.00% Coverage (81.80% Estimated after merge)
0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: edu.ie3:simona

View in SonarQube

sebastian-peter · 2022-04-11T20:33:30Z

This branch was just way out of sync with dev. A merge fixed everything.

stale review

Fix config auto-generation

679cd4c

ckittl added the bug Something isn't working label Feb 8, 2022

ckittl added this to the Version 3.0 milestone Feb 8, 2022

ckittl requested a review from a team February 8, 2022 13:10

ckittl self-assigned this Feb 8, 2022

sebastian-peter requested changes Feb 9, 2022

View reviewed changes

gradle/scripts/tscfg.gradle Show resolved Hide resolved

gradle/scripts/tscfg.gradle Outdated Show resolved Hide resolved

This was referenced Feb 9, 2022

DBFS with multiple connections between sub grids #25

Merged

Add config possibility for transformer control groups #91

Closed

ckittl added 3 commits February 17, 2022 16:09

Addressing reviewer's comments

6c05e0c

Merge remote-tracking branch 'origin/dev' into ck/#130-fixConfigAutoG…

bbbc610

…eneration

Fix broken tests

cea4fa4

This comment has been minimized.

Sign in to view

ckittl requested a review from sebastian-peter February 17, 2022 15:45

sebastian-peter previously approved these changes Feb 17, 2022

View reviewed changes

Fix config sample generation

999ec92

ckittl dismissed sebastian-peter’s stale review via 999ec92 February 22, 2022 09:26

This comment has been minimized.

Sign in to view

ckittl requested a review from sebastian-peter February 22, 2022 09:45

ckittl mentioned this pull request Feb 22, 2022

Consolidate csv parameterization #150

Merged

johanneshiry previously requested changes Feb 22, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

ckittl and others added 3 commits March 8, 2022 10:56

Update src/main/scala/edu/ie3/simona/config/ConfigFailFast.scala

5509d0b

Co-authored-by: Johannes Hiry <johannes.hiry@tu-dortmund.de>

Update src/main/scala/edu/ie3/simona/config/ConfigFailFast.scala

d4d002c

Co-authored-by: Johannes Hiry <johannes.hiry@tu-dortmund.de>

Merge remote-tracking branch 'origin/dev' into ck/#130-fixConfigAutoG…

4539f85

…eneration

ckittl added 2 commits March 8, 2022 12:26

Fix config sample generation on Windows OS

ecbb226

by choosing the OS dependent temporary file path

Fix test

d42d174

This comment has been minimized.

Sign in to view

ckittl requested a review from johanneshiry March 8, 2022 12:37

sebastian-peter mentioned this pull request Mar 21, 2022

Remove workaround for tscfg tmp directory #178

Closed

sebastian-peter approved these changes Mar 21, 2022

View reviewed changes

ckittl requested review from t-ober and danielfeismann March 24, 2022 11:42

t-ober self-assigned this Apr 10, 2022

t-ober approved these changes Apr 11, 2022

View reviewed changes

t-ober self-requested a review April 11, 2022 13:06

t-ober requested changes Apr 11, 2022

View reviewed changes

Merge branch 'dev' into ck/#130-fixConfigAutoGeneration

97e551a

sebastian-peter requested a review from t-ober April 11, 2022 20:32

t-ober approved these changes Apr 12, 2022

View reviewed changes

t-ober merged commit 6348f8c into dev Apr 12, 2022

t-ober deleted the ck/#130-fixConfigAutoGeneration branch April 12, 2022 05:42

dependabot bot mentioned this pull request Nov 20, 2023

Bump com.github.ie3-institute:simonaAPI from 0.2.0 to 0.3.0 #655

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix config auto-generation #131

Fix config auto-generation #131

ckittl commented Feb 8, 2022 •

edited

Loading

johanneshiry commented Feb 8, 2022 •

edited

Loading

sebastian-peter left a comment •

edited

Loading

This comment has been minimized.

codecov bot commented Feb 17, 2022 •

edited

Loading

This comment has been minimized.

sebastian-peter commented Feb 22, 2022 •

edited

Loading

johanneshiry left a comment

johanneshiry Feb 22, 2022

ckittl Mar 8, 2022

This comment has been minimized.

ckittl commented Mar 8, 2022

This comment has been minimized.

ckittl commented Mar 8, 2022

ckittl commented Mar 11, 2022

sebastian-peter left a comment

t-ober left a comment

t-ober left a comment

sonarqubegithubprchecks bot commented Apr 11, 2022

sebastian-peter commented Apr 11, 2022

Fix config auto-generation #131

Fix config auto-generation #131

Conversation

ckittl commented Feb 8, 2022 • edited Loading

johanneshiry commented Feb 8, 2022 • edited Loading

sebastian-peter left a comment • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

codecov bot commented Feb 17, 2022 • edited Loading

Codecov Report

This comment has been minimized.

sebastian-peter commented Feb 22, 2022 • edited Loading

johanneshiry left a comment

Choose a reason for hiding this comment

johanneshiry Feb 22, 2022

Choose a reason for hiding this comment

ckittl Mar 8, 2022

Choose a reason for hiding this comment

This comment has been minimized.

ckittl commented Mar 8, 2022

This comment has been minimized.

ckittl commented Mar 8, 2022

ckittl commented Mar 11, 2022

sebastian-peter left a comment

Choose a reason for hiding this comment

t-ober left a comment

Choose a reason for hiding this comment

t-ober left a comment

Choose a reason for hiding this comment

sonarqubegithubprchecks bot commented Apr 11, 2022

Analysis Details

0 Issues

Coverage and Duplications

sebastian-peter commented Apr 11, 2022

ckittl commented Feb 8, 2022 •

edited

Loading

johanneshiry commented Feb 8, 2022 •

edited

Loading

sebastian-peter left a comment •

edited

Loading

codecov bot commented Feb 17, 2022 •

edited

Loading

sebastian-peter commented Feb 22, 2022 •

edited

Loading