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.

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

CDA can't handle Timezones that don't exist anymore (or at all) #987

Open
DanielTOsborne opened this issue Dec 19, 2024 · 4 comments
Open
Labels
bug Something isn't working good first issue Good for newcomers priority:medium We care, and will try and get to it soon.

Comments

@DanielTOsborne
Copy link
Collaborator

DanielTOsborne commented Dec 19, 2024

The CWMS schema contains timezones that either never existed (placeholders), or no longer exist in the host system (java runtime).
As such, any attempt to retrieve locations fails with the following:

18-Dec-2024 23:55:55.954 SEVERE [https-openssl-nio-8243-exec-48] cwms.cda.api.LocationController.getAll 7896909191952785378: failed to process request
        java.time.zone.ZoneRulesException: Unknown time-zone ID: US/Pacific-New
                at java.time.zone.ZoneRulesProvider.getProvider(ZoneRulesProvider.java:272)
                at java.time.zone.ZoneRulesProvider.getRules(ZoneRulesProvider.java:227)
                at java.time.ZoneRegion.ofId(ZoneRegion.java:120)
                at java.time.ZoneId.of(ZoneId.java:411)
                at java.time.ZoneId.of(ZoneId.java:359)
                at cwms.cda.data.dao.LocationsDaoImpl.buildLocation(LocationsDaoImpl.java:147)
                at org.jooq.RecordMapper.apply(RecordMapper.java:87)
                at org.jooq.RecordMapper.apply(RecordMapper.java:72)
                at java.util.stream.Collectors.lambda$mapping$8(Collectors.java:355)
                at java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
                at java.util.Iterator.forEachRemaining(Iterator.java:116)
                at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
                at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
                at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
                at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
                at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
                at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
                at org.jooq.impl.AbstractCursor.collect(AbstractCursor.java:78)
                at org.jooq.impl.ResultQueryTrait.collect(ResultQueryTrait.java:361)
                at org.jooq.impl.ResultQueryTrait.fetch(ResultQueryTrait.java:1465)
                at cwms.cda.data.dao.LocationsDaoImpl.getLocations(LocationsDaoImpl.java:125)
                at cwms.cda.api.LocationController.getAll(LocationController.java:198)

In the CWMS schema, there are several timezone entries that trigger this:

  • US/Pacific-New
  • Unknown or Not Applicable
    There may be others, those are just the two I encountered.

{74755189-8686-4279-863E-1DFBE2676C31}

Code example uses cwms-python, but that shouldn't matter, just make a request to the /locations endpoint using an internal CDA instance (not the national/public):

#!python3

import cwms
import pandas as pd

#this code changes the CDA instance that you are connected to 
apiRoot = "https://url.internal/spk-data/"
cwms.api.init_session(api_root=apiRoot)

locations = cwms.get_locations('SPK')

print(locations.df)

Expectations
I don't know why it's trying to convert the timezone for location retrieval, when it's not even needed.
I'd expect the /locations endpoint to work fine, perhaps printing a warning in the log, not erroring out entirely.

@MikeNeilson
Copy link
Contributor

It's trying to convert because the date types aren't strings internally, so we can validate user input. However; in this case we clearly forgot we store a bunch of odd edge cases.

All of those should be changed from US/Pacific-New as that was never actually adopted and it's an official timezone, but in that particular case I think we should also work around it in CDA.

@MikeNeilson MikeNeilson added bug Something isn't working good first issue Good for newcomers priority:medium We care, and will try and get to it soon. labels Dec 19, 2024
@MikeNeilson
Copy link
Contributor

That reminds me, there's also some "USGS", or maybe it was SHEF, timezones we support by name as well. They should also be translated.

@perrymanmd any other time zone in database edge cases we've forgotten about?

@perrymanmd
Copy link

The SHEF time zones and their equivalents used by the python SHEF parser are shown below.

    TZ_NAMES = {
        #
        # Not modified by SHEFPARM file
        #
        'J'  : "PRC",                              # China

        "HS" : "US/Hawaii",                        # Hawaiian standard
        "HD" : "US/Hawaii",                        # Hawaiian daylight
        'H'  : "US/Hawaii",                        # Hawaiian local

        "BS" : "Etc/GMT+11",                       # Bering standard       (obsolete, Aleutian Islands now use US/Alaska)
        "BD" : "Etc/GMT+10",                       # Bering daylight       (obsolete, Aleutian Islands now use US/Alaska)
        'B'  : "Pacific/Midway",                   # Bering local          (obsolete, Aleutian Islands now use US/Alaska)

        "LS" : "Etc/GMT+9",                        # Alaskan standard
        "LD" : "Etc/GMT+8",                        # Alaskan daylight
        'L'  : "US/Alaska",                        # Alaskan local

        "YS" : "Etc/GMT+8",                        # Yukon standard        (--shefit_times gives bad times always)
        "YD" : "Etc/GMT+7",                        # Yukon daylight        (--shefit_times gives bad times always)
        'Y'  : "Canada/Yukon",                     # Yukon local           (--shefit_times gives bad times always)

        "PS" : "Etc/GMT+8",                        # Pacific standard
        "PD" : "Etc/GMT+7",                        # Pacific daylight
        'P'  : "US/Pacific",                       # Pacific local

        "MS" : "Etc/GMT+7",                        # Mountain standard
        "MD" : "Etc/GMT+6",                        # Mountain daylight
        'M'  : "US/Mountain",                      # Mountain local

        "CS" : "Etc/GMT+6",                        # Central standard
        "CD" : "Etc/GMT+5",                        # Central daylight
        'C'  : "US/Central",                       # Central

        "ES" : "Etc/GMT+5",                        # Eastern standard
        "ED" : "Etc/GMT+4",                        # Eastern daylight
        'E'  : "US/Eastern",                       # Eastern local

        "AS" : "Etc/GMT+4",                        # Atlantic standard
        "AD" : "Etc/GMT+3",                        # Atlantic daylight
        'A'  : "Canada/Atlantic",                  # Atlantic local

        "NS" : "timedelta(hours=-3, minutes=-30)", # Newfoundland standard
        "ND" : "timedelta(hours=-2, minutes=-30)", # Newfoundland daylight (--shefit_times gives bad times always)
        'N'  : "Canada/Newfoundland",              # Newfoundland local    (--shefit_times gives bad times in summer)

        'Z'  : "UTC"}                              # Zulu

@perrymanmd
Copy link

It seems like @ktarbet ran across "AST" in USGS data for Puerto Rico and it was being treated as Alaska Standard Time, but I don't remember the details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers priority:medium We care, and will try and get to it soon.
Projects
None yet
Development

No branches or pull requests

3 participants