Change get / update dataset metadata to handle two more metadata fields by jmasukawa · Pull Request #958 · Kaggle/kaggle-cli

jmasukawa · 2026-04-07T08:57:11Z

Leverages changes to DatasetApiService/V1/UpdateDatasetMetadataHandler and DatasetApiService/V1/GetDatasetMetadataHandler to accept expected update frequency and user specified sources.

Wait before merging:

MT changes are deployed: https://github.com/Kaggle/kaggleazure/pull/41349
Correct set of kagglesdk changes are made / merged and release is pushed: Update protos and release 0.1.17 kaggle-sdk-python#31

Other changes:

Pin virtualenv version so ./docker-hatch works
Run lint / format before making any changes (there was drift)
Docs updates for datasets / datasets metadata

Local testing:

screencast

cc: @goeffthomas

http://b/500108129

jplotts

Thanks! Just a few questions / comments, but overall LGTM.

jplotts · 2026-04-07T14:08:14Z

        with tempfile.TemporaryDirectory() as tmpdir:
            meta_file = os.path.join(tmpdir, "kernel-metadata.json")
-            (fd, code_file) = tempfile.mkstemp("code", "py", tmpdir, text=True)
+            fd, code_file = tempfile.mkstemp("code", "py", tmpdir, text=True)


j/w - did you do these formatting changes manually? I know that @stevemessick has some autoformatting set up in this repo, so just want to make sure we're not messing with that.

yep, this is from the tooling. it came from ./docker-hatch run lint:fmt which should be the correct tool path.

something i did notice is that if someone else uses hatch run lint:fmt then it will use their machine's python version, which might format differently.

i used the docker one because i assumed it was the way we keep it consistent. but, the docker python version is a bit outdated. so i'm guessing what happened is someone used the linter w/local python instead of docker one time, and it slipped in.

jplotts · 2026-04-07T14:10:17Z

+        ]:
+            return expected_update_frequency
+
+        return "not specified"


Should we notify the user in any way?

hm, yeah that's a good point. i wrote this before i wrote the MT code, but i think instead, we should not try to sanitize their input and just let the MT fail with the nice user-facing error message from the MT (which shows the list of valid values)

jplotts · 2026-04-07T14:11:27Z

+        if not expected_update_frequency:
+            return "not specified"
+
+        if expected_update_frequency in [


Not sure about this; now the canonical list is stored in 2 places. What about just sending the bad value off to the MT to be rejected?

yeah i agree. i just had the same thought when responding to your comment above. i wrote the CLI code first before the MT code, and didn't think about it.

will change to your suggestion.

jmasukawa · 2026-04-07T09:01:23Z

-  ]
+  ],
+  "expectedUpdateFrequency": "monthly",
+  "userSpecifiedSources": "World Bank and OECD ([link](http://data.worldbank.org/indicator/NY.GDP.MKTP.CD))",


This is the internal field name. open to better alternatives.

I had wanted to just call it sources, but given we follow the Data Package format as much as possible, it conflicts with their "sources". which are objects that have a Title (required) and Path (optional): https://specs.frictionlessdata.io//data-package/#sources

However, DatasetVersion.UserSpecifiedSources is a markdown string, so that's not feasible to turn into an array of objects and back, without some heavy assumptions that could be wrong (comma separation, etc).

It seemed like too much work atm to refactor how we store UserSpecifiedSources to use a collection of objects just for this, so i went with a name that was significantly deviated from the Data Package spec to make it clear it's something else.

Open to other suggestions.

@goeffthomas for your thoughts here, given you mentioned Data Package adherence

Thanks for laying all of that out. +1 to your assessment and naming. I think my earlier comment about adhering and using this property was before knowing how they recommend extending. So yeah, if we want something that more matches how we let users specify the sources, this all makes sense to me.

Thanks for the feedback! SG, i'll continue on this way to match our UI

jmasukawa · 2026-04-07T09:02:37Z

+            "weekly",
+            "daily",
+            "hourly",
+            "unspecified",  # Some datasets have this still, but it is not canonical


there's only 94 datasets using unspecified instead of not specified, but it felt wrong that you could theoretically pull metadata down via get, which you couldn't push back up in update: http://screen/AU3qjn9KnaWYyfk

We should probably do a data migration to get things to line up to the enum, then migrate that column to an enum. filed a ticket for it: http://b/500450622

jmasukawa · 2026-04-07T21:31:01Z

        with tempfile.TemporaryDirectory() as tmpdir:
            meta_file = os.path.join(tmpdir, "kernel-metadata.json")
-            (fd, code_file) = tempfile.mkstemp("code", "py", tmpdir, text=True)
+            fd, code_file = tempfile.mkstemp("code", "py", tmpdir, text=True)


yep, this is from the tooling. it came from ./docker-hatch run lint:fmt which should be the correct tool path.

something i did notice is that if someone else uses hatch run lint:fmt then it will use their machine's python version, which might format differently.

i used the docker one because i assumed it was the way we keep it consistent. but, the docker python version is a bit outdated. so i'm guessing what happened is someone used the linter w/local python instead of docker one time, and it slipped in.

jmasukawa · 2026-04-07T21:32:21Z

+        ]:
+            return expected_update_frequency
+
+        return "not specified"


hm, yeah that's a good point. i wrote this before i wrote the MT code, but i think instead, we should not try to sanitize their input and just let the MT fail with the nice user-facing error message from the MT (which shows the list of valid values)

jmasukawa · 2026-04-07T21:33:09Z

+        if not expected_update_frequency:
+            return "not specified"
+
+        if expected_update_frequency in [


yeah i agree. i just had the same thought when responding to your comment above. i wrote the CLI code first before the MT code, and didn't think about it.

will change to your suggestion.

Created with: `pip-compile pyproject.toml -o requirements.lock`. `kagglesdk` dep was bumped from #958 (needed to `pip install piptools` w/ --break-system-packages but that seems fine)

jmasukawa added 4 commits April 6, 2026 22:43

Pin virtualenv so docker-hatch keeps working

7631ce9

Run ./docker-hatch run lint:fmt to reformat lint drift

91f47a9

Update datasets metadata command docs for --update to be clear

43232ab

Enable get / update dataset metadata to handle two more fields.

334381e

jmasukawa force-pushed the datasets-create-more-metadata branch from 2272478 to 334381e Compare April 7, 2026 09:06

jmasukawa requested a review from jplotts April 7, 2026 09:10

jplotts approved these changes Apr 7, 2026

View reviewed changes

jmasukawa commented Apr 7, 2026

View reviewed changes

jmasukawa added 2 commits April 7, 2026 21:43

PR Feedback: Defer to backend for expected freq values

6a513a3

PR Feedback: Fix server error printing in update metadata path

3c51b38

jmasukawa mentioned this pull request Apr 8, 2026

Kaggle CLI: Dataset image upload via datasets metadata --update #959

Merged

Bump kagglesdk version

23ce0cf

jmasukawa merged commit 1dca025 into main Apr 8, 2026
5 checks passed

jmasukawa deleted the datasets-create-more-metadata branch April 8, 2026 22:29

jmasukawa mentioned this pull request Apr 9, 2026

Regenerate requirements.lock from recent kagglesdk bump #963

Merged

Conversation

jmasukawa commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jplotts left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmasukawa Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmasukawa Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmasukawa Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmasukawa Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmasukawa commented Apr 7, 2026 •

edited

Loading

jmasukawa Apr 7, 2026 •

edited

Loading

jmasukawa Apr 7, 2026 •

edited

Loading

jmasukawa Apr 7, 2026 •

edited

Loading

jmasukawa Apr 7, 2026 •

edited

Loading