Skip to content

fix: parse whole-second protobuf duration strings like 0s#58

Merged
stevemessick merged 1 commit into
Kaggle:mainfrom
aadi-joshi:fix/timedelta-whole-second-durations
Jul 1, 2026
Merged

fix: parse whole-second protobuf duration strings like 0s#58
stevemessick merged 1 commit into
Kaggle:mainfrom
aadi-joshi:fix/timedelta-whole-second-durations

Conversation

@aadi-joshi

Copy link
Copy Markdown
Contributor

Summary

Fix TimeDeltaSerializer._from_dict_value to accept protobuf JSON duration strings without a fractional component (e.g. 0s, 151s). The current implementation always splits on ., which crashes quota and other API responses that return whole-second durations.

Needs manual sync with the kapigen base.

Root cause

(seconds, nanosRaw) = value.rstrip("s").split(".")

Protobuf JSON allows whole-second values like 0s. When .split(".") returns one element, unpacking fails with ValueError: not enough values to unpack (expected 2, got 1).

Reported in Kaggle/kaggle-cli#1064.

Tests

Added TimeDeltaSerializerTests in kaggle_object_tests.py:

  • whole-second durations (0s, 151s)
  • fractional durations (151.500s) regression
  • ApiGetAcceleratorQuotaStatisticsResponse JSON deserialization

Test plan

  • TimeDeltaSerializer._from_dict_value("0s") returns timedelta(0)
  • CI: python -m unittest kaggle_object_tests.TimeDeltaSerializerTests

Needs manual sync with the kapigen base.
aadi-joshi added a commit to aadi-joshi/kaggle-cli that referenced this pull request Jun 30, 2026
@aadi-joshi

Copy link
Copy Markdown
Contributor Author

Following up on stevemessick's guidance in Kaggle/kaggle-cli#1082. Marked for manual sync with the kapigen base. The auth portion of Kaggle/kaggle-cli#1064 is handled separately in #1082.

stevemessick pushed a commit to Kaggle/kaggle-cli that referenced this pull request Jul 1, 2026
## Summary

Fix `_is_help_or_version_command` so subcommand `-v` (the CSV output
alias on commands like `quota`) no longer skips authentication.
Top-level `kaggle -v` / `kaggle --version` still skip auth as before.

The duration parsing crash from #1064 is handled separately in
Kaggle/kaggle-sdk-python#58 (needs kapigen sync and a new kagglesdk
release).

## Root cause

`authenticate()` runs before argparse and joins `sys.argv[1:]` into a
string. `"quota -v".endswith("-v")` matched the global version check
even though `-v` here is the CSV flag, causing unauthenticated API calls
and 401s.

## Tests

Added `tests/unit/test_help_version_auth.py` covering top-level
version/help, subcommand CSV `-v`, and subcommand help.

## Test plan

- [x] `pytest tests/unit/test_help_version_auth.py`
- [ ] Maintainer: please run `/gcbrun` for Cloud Build (fork PR)

Related to #1064

@stevemessick stevemessick left a comment

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.

Thanks!

@stevemessick stevemessick merged commit ab06cbb into Kaggle:main Jul 1, 2026
7 checks passed
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.

2 participants