Skip to content

Add operationalPeriod to OTC station flow#366

Merged
jonathanthiry merged 2 commits into
masterfrom
operational-period-otc-station
May 21, 2026
Merged

Add operationalPeriod to OTC station flow#366
jonathanthiry merged 2 commits into
masterfrom
operational-period-otc-station

Conversation

@jonathanthiry
Copy link
Copy Markdown
Contributor

OTC will be able to provide an operational period string in https://meta.icos-cp.eu/edit/otcentry/ which will end up in the production metadata https://meta.icos-cp.eu/edit/icosmeta/. The property will be visible on the station pages and available in SPARQL using the http://meta.icos-cp.eu/ontologies/cpmeta/hasOperationalPeriod predicate.

Copy link
Copy Markdown
Contributor

@Jonathan-Schenk Jonathan-Schenk 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 to me! I do not have any comment on this.

Copy link
Copy Markdown
Contributor

@ggVGc ggVGc left a comment

Choose a reason for hiding this comment

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

Seems fine, just a small comment, which I think applies to several other similar cases in the code-base, so maybe something to look at as a future improvement (improving the access to StationSpecifics data).

Comment on lines +185 to +189
@operationalPeriod = @{
station.specificInfo match {
case sites: SitesStationSpecifics => sites.operationalPeriod
case otc: OtcStationSpecifics => otc.operationalPeriod
case _ => None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is a bit of a code-smell, where we set up a future potential issue where another StationSpecifics gets an operationalPeriod, everything builds fine, but it is not presented on this page. My first thought is that we should not have the catch-all clause, but after a quick try, it seems I did not get an exhaustiveness error if I leave out the catchall clause, possibly because it's a .scala.html file?

However, in this case, since operationalPeriod is a Maybe anyway, I think we could just make it part of the StationSpecifics trait?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I usually test that I get the result that I want on the page, but I see your point that it could be better to add operationalPeriod to the trait and be sure that the page always work as intended.

Removing the catch-all case would throw a runtime exception for other station types. This would throw a warning but, for some reason, we have disabled them for HTML files. I'll create a task to see if we could enable them and fix the warnings.

@jonathanthiry jonathanthiry merged commit 51e23d7 into master May 21, 2026
1 check passed
@jonathanthiry jonathanthiry deleted the operational-period-otc-station branch May 21, 2026 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants