SEPURI-SAI-KRISHNA opened a new pull request, #45260:
URL: https://github.com/apache/superset/pull/45260
### SUMMARY
Fixes #45259.
Two `HOLIDAY()` time ranges returned a 500. Both now return what they should.
**Giving the country without the datetime.** Both arguments after the holiday
name are optional in the grammar, so `holiday("Christmas Day", "UK")` parses.
`eval` read the second argument as the datetime whatever it held, so it took
`.year` off a string and the request died with
`'str' object has no attribute 'year'`.
The two kinds of argument are distinguishable: a datetime is always a
function
call (`datetime(...)`, `dateadd(...)`, ...) and a country is always quoted
text.
Resolving the optional arguments by kind rather than by position makes the
two-argument form work instead of crashing, and leaves the three documented
forms unchanged.
**A country with no calendar.** `country_holidays` raises
`NotImplementedError`
for a country it has no calendar for. That is the caller's to correct, so it
now
arrives as a `ValueError` naming the country, which the
`/api/v1/time_range/` handler already maps to a 400, the same way an unknown
holiday does. The accepted spellings are the ISO code and the package's
registered name, so `"us"` (right code, wrong case) and `"United Kingdom"`
(written out in full) both used to 500.
Case is not normalised: registered names such as `"Germany"` are not
uppercase,
so upper-casing the argument would break inputs that work today. The error
message is the fix for those.
The in-UI syntax help gains the two-argument form it now supports.
I swept 271 grammar-valid expressions across `DATETIME`, `DATEADD`,
`DATEDIFF`,
`DATETRUNC`, `LASTDAY` and `HOLIDAY`. These two were the only ones raising
anything other than `ValueError`, and after the change the sweep raises
nothing
but `ValueError`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
```
GET /api/v1/time_range/?q='HOLIDAY("Boxing Day", "UK") :
DATETIME("2027-01-01")'
500 {"error": "'str' object has no attribute 'year'"}
GET /api/v1/time_range/?q='HOLIDAY("Christmas Day", DATETIME("2019"), "us")
: now'
500 {"error": "Country us not available"}
```
After:
```
GET /api/v1/time_range/?q='HOLIDAY("Boxing Day", "UK") :
DATETIME("2027-01-01")'
200 {"result": [{"since": "2026-12-26T00:00:00", "until":
"2027-01-01T00:00:00", ...}]}
GET /api/v1/time_range/?q='HOLIDAY("Christmas Day", DATETIME("2019"), "us")
: now'
400 {"message": "Unexpected time range: Unable to find such a country: [us]"}
```
### TESTING INSTRUCTIONS
Explore any chart with a temporal column, open the Time Range control,
switch to
Advanced, and set START to `HOLIDAY("Boxing Day", "UK")` and END to
`DATETIME("2027-01-01")`. The preview resolves to 2026-12-26 rather than
failing. Setting a country such as `United Kingdom` reports that no such
country
was found instead of failing.
New unit tests:
```
pytest tests/unit_tests/utils/date_parser_tests.py -k holiday
```
`test_holiday_reads_a_country_given_without_a_datetime` uses Boxing Day,
which
the UK calendar has and the default US calendar does not, so it fails if the
country argument is read but ignored as well as if it crashes.
Full file: `pytest tests/unit_tests/utils/date_parser_tests.py` passes.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #45259
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
The UI change is two lines added to the time-range function help: the
`holiday([holiday string], [country name])` syntax line and a matching
example.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
