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

More expressive primary service init failure #476

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
+28 −9
Open

More expressive primary service init failure #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
baadd69
more expressive primary service init failure
t-ober Mar 1, 2023
a381313
Merge branch 'dev' into to/#475-expressice-dtp-exception
t-ober Mar 1, 2023
f602105
Merge branch 'dev' into to/#475-expressice-dtp-exception
sebastian-peter Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
- Add safety factor sRated calculation [#629](https://github.com/ie3-institute/simona/issues/629)
- Re-implemented ResultEventListener in akka typed [#343](https://github.com/ie3-institute/simona/issues/343)
- More expressive failure message upon setting up Primary Service with wrong time pattern [#475](https://github.com/ie3-institute/simona/issues/475)

### Changed
- Adapted to changed data source in PSDM [#435](https://github.com/ie3-institute/simona/issues/435)
Expand Down
1 change: 1 addition & 0 deletions input/samples/vn_simona/vn_simona.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ simona.input.primary.csvParams = {
directoryPath: "input/samples/vn_simona/fullGrid"
csvSep: ","
isHierarchic: false
timePattern: "yyyy-MM-dd'T'HH:mm:ss[.S[S][S]]'Z'"
Copy link
Member

Choose a reason for hiding this comment

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

That's the default time pattern, so this line effectively does not change any configuraiton. Does it make sense to include it?

}
simona.input.grid.datasource.id = "csv"
simona.input.grid.datasource.csvParams = {
Expand Down
35 changes: 26 additions & 9 deletions src/main/scala/edu/ie3/simona/service/primary/PrimaryServiceWorker.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ import edu.ie3.simona.service.{ServiceStateData, SimonaService}
import edu.ie3.simona.util.TickUtil.{RichZonedDateTime, TickLong}
import edu.ie3.util.scala.collection.immutable.SortedDistinctSeq

import java.io.IOException
import java.nio.file.Path
import java.time.ZonedDateTime
import java.time.format.DateTimeParseException
import java.util.UUID
import scala.jdk.CollectionConverters._
import scala.jdk.OptionConverters.RichOptional
Expand Down Expand Up @@ -78,15 +80,30 @@ final case class PrimaryServiceWorker[V <: Value](
Try {
/* Set up source and acquire information */
val factory = new TimeBasedSimpleValueFactory(valueClass, timePattern)
val source = new CsvTimeSeriesSource(
csvSep,
directoryPath,
fileNamingStrategy,
timeSeriesUuid,
filePath,
valueClass,
factory
)
val source = Try(
new CsvTimeSeriesSource(
csvSep,
directoryPath,
fileNamingStrategy,
timeSeriesUuid,
filePath,
valueClass,
factory
)
) match {
Copy link
Member

Choose a reason for hiding this comment

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

This error matching would also be nice for SqlTimeSeriesSource as well, no? So maybe we can move this an abstraction level upwards, i.e. checking for Failure after *TimeSeriesSource has been created.

case Failure(exc: DateTimeParseException) =>
throw new IOException(
s"Could not initiate the time series source. Parsing ${exc.getParsedString} failed with timepattern: $timePattern. The time pattern can be configured in the configuration.",
exc
)
Comment on lines +94 to +98
Copy link
Member

Choose a reason for hiding this comment

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

If we want this kind of distinction, would'nt it rather make sense to have it in PSDM?
So, everything that can go wrong within *TimeSeriesSource is fully described by an exception that its constructor throws. Then, when we arrive at the level of PrimaryServiceWorker, we can then wrap the exception once more and say: "During creation of the timeseries source, an error occured." or something like that.

Copy link
Member

@sebastian-peter sebastian-peter Feb 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Possibly solved here (or with an earlier PR). We should still check if DateTimeParseException is also wrapped in a SourceException within PSDM

case Failure(exc) =>
throw new IOException(
s"Could not initiate the time series source.",
exc
)
case Success(source) =>
source
}
(source, simulationStart)
}

Expand Down