Skip to content

fix: migrate validation models to Pydantic V2 - #315

Open
dulcetberg wants to merge 1 commit into
hotosm:developfrom
dulcetberg:fix/pydantic-v2-migration
Open

dulcetberg wants to merge 1 commit into
hotosm:developfrom
dulcetberg:fix/pydantic-v2-migration

Conversation

@dulcetberg

Copy link
Copy Markdown

What type of PR is this?

Check all applicable

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation
  • 🧑‍💻 Refactor
  • ✅ Test
  • 🤖 Build or CI
  • ❓ Other (please specify)

Related Issue :

None. This replaces my own PRs #307, #308, #309, #310 and #311. I'll close those once this is open.

What does this PR do ?

Moves the last Pydantic V1 constructs in src/validation/models.py over to V2:

  • 9 @validator become @field_validator + @classmethod
  • 4 class Config become model_config = ConfigDict(...)
  • the 3 cross-field validators read info.data instead of values
  • pre=True, always=True becomes mode="before" plus validate_default=True on the field
  • the validator import is dropped, since nothing uses it now

That file is the only place in the repo with these constructs, so there are none left after this. The other modules importing pydantic (API/auth/, API/download_metrics.py, API/custom_exports.py) only use plain BaseModel, Field and ValidationError, which didn't change between V1 and V2.

Nothing breaks today either way. The repo already runs Pydantic 2.x because geojson-pydantic==1.0.1 requires it, so these currently work through compatibility shims. The shims are marked "Deprecated in Pydantic V2.0 to be removed in V3.0", so this is about not being stuck when a V3 bump comes up.

Consideration :

Why this is one PR and not the five I opened before. I split it originally thinking smaller PRs were easier to review. That was wrong. All five came off the same commit and all five edited the same import line, so merging any one would have put the other four in conflict straight away. And each one on its own left the file half migrated, still importing validator next to field_validator, which is what @spwoodcock flagged on #307. One commit for the whole file is less work to review, not more.

Scope. Only src/validation/models.py, and only the V1 to V2 constructs. I didn't reformat anything else.

Left out on purpose. There are 27 more deprecation warnings from Field(example=...), which V2 wants as json_schema_extra. That's a big noisy diff and it changes the generated OpenAPI docs, so it isn't in here. Happy to do it separately if it's wanted.

On info.data ordering. This is the part I'd check first if I were reviewing. In V2 info.data only holds fields validated before the current one, so the three cross-field validators only work if the declaration order cooperates. It does, in all three:

  • check_bind_option validates bind_zip, declared in RawDataCurrentParams, and reads output_type, declared in the parent RawDataCurrentParamsBase. Parent fields are ordered first, so output_type is there.
  • Both set_geometry_or_iso3 validators read iso3, and the DynamicCategoriesModel one also reads dataset, hdx_upload and categories. In both models geometry is declared after all of them.

The behaviour checks below cover this rather than leaving it to my reading of the code.

How to test ?

I couldn't run the full py.test -v -s against a populated OSM database, so I checked the part I changed directly instead of reporting a pass I didn't see.

The validators in this file can be exercised without a database or GDAL. I imported the models before and after and ran the same checks against both:

pydantic 2.12.3

DEPRECATION WARNINGS AT IMPORT
                               before    after
  V1 @validator                     9        0
  class-based Config                4        0
  Field example= (not this PR)     27       27
  other                             2        0
  TOTAL                            42       27
  in scope for this PR             13        0

BEHAVIOUR CHECKS (must be identical)
  alias: camelCase in                    ok     -> ok     same
  alias: snake_case in                   ok     -> ok     same
  bind_zip: geojson streams              ok     -> ok     same
  bind_zip: shp cannot stream            ok     -> ok     same
  category: bad format                   ok     -> ok     same
  category: bad geom type                ok     -> ok     same
  geometry: multi-feature FC rejected    ok     -> ok     same
  geometry: single-feature FC unwraps    ok     -> ok     same
  geometry_type: de-duplicated           ok     -> ok     same
  hdx: allowed tag                       ok     -> ok     same
  hdx: rejected tag                      ok     -> ok     same
  schema: SnapshotResponse example       ok     -> ok     same
  stats: both                            ok     -> ok     same
  stats: geometry only                   ok     -> ok     same
  stats: iso3 only                       ok     -> ok     same
  stats: neither                         ok     -> ok     same

  before: 16/16 passed
  after:  16/16 passed

RESULT: PASS

That covers both cross-field validators, the camelCase alias generator and populate_by_name, the HDX tag and category type/format validators, the FeatureCollection handling in GeometryValidatorMixin, geometry_type de-duplication, and that json_schema_extra still reaches the schema. I can turn these into a pytest module under tests/ if you'd want that in the repo. They don't need a database, so they'd run in CI cleanly.

Two CI results that aren't from this PR

  • Unit Test. develop itself is currently 2 failed, 37 passed. Both failures are test_stats_endpoint_custom_polygon and test_stats_endpoint_iso3, both connection timeouts to apps.kontur.io from the runner rather than assertion failures. Expect the same two here.
  • black. black --check . fails on unmodified develop for 4 files (API/raw_data.py, src/app.py, src/query_builder/builder.py, src/validation/models.py). The action tracks latest black and the repo was formatted with an older version. I didn't run black over the file, because that would mix a reformat-everything diff into a behaviour change. Say the word if you'd rather I did, or if a separate formatting PR would be useful.

Screenshots :

Screenshot 2026-09-21 at 1 17 12 PM

AI Tool Usage

How much of this PR was AI-assisted? (check one)

  • 0 - No AI at any point
  • 1 - AI helped me think, but I wrote all the code myself
  • 2 - I planned the change and decided the approach; AI helped write the code
  • 3 - AI planned and wrote it; I checked and approved each step as it went
  • 4 - I set the AI going and left it to it; I reviewed the finished result
  • 5 - I set the AI going and left it to it; nobody has read the result - no review, or AI review only

If level 1 or above:

  • Tool(s) used: Claude Code
  • What was generated: the consolidated commit and the before/after verification harness.
  • What you reviewed and changed: I read the diff hunk by hunk against the V2 migration guide, and checked the info.data ordering argument above myself instead of taking it on trust. I decided to drop the five way split and to leave the Field(example=...) deprecations out of scope.

Checklists:

Checklist before requesting review :

  • 📖 Read the HOT Code of Conduct: https://docs.hotosm.org/code-of-conduct

  • 👷‍♀️ Create small PRs. In most cases, this will be possible.

  • ✅ Provide tests for your changes.

  • 📝 Use descriptive commit messages.

  • 📗 Update any related documentation and include any relevant screenshots.

  • 📖 Read the HOT Code of Conduct: https://docs.hotosm.org/code-of-conduct

  • 👷‍♀️ Create small PRs. In most cases, this will be possible.

  • ✅ Provide tests for your changes.

  • 📝 Use descriptive commit messages.

  • 📗 Update any related documentation and include any relevant screenshots.

[optional] What gif best describes this PR or how it makes you feel?

Replaces every Pydantic V1 construct in src/validation/models.py with its
V2 equivalent:

- 9 `@validator` -> `@field_validator` + `@classmethod`
- 4 `class Config` -> `model_config = ConfigDict(...)`
- cross-field validators take `info: ValidationInfo`, reading `info.data`
  instead of `values`
- `pre=True, always=True` -> `mode="before"` plus `validate_default=True`
  on the corresponding Field
- drops the now-unused `validator` import

Supersedes the five split PRs (hotosm#307, hotosm#308, hotosm#309, hotosm#310, hotosm#311), which each
touched the same import line and so could not be merged independently.
@dulcetberg

Copy link
Copy Markdown
Author

Two corrections to the CI notes in my description, both of which I got wrong.

black passed. I based that bullet on black --check . failing against unmodified develop with the black version installed locally. The action pins an older black that matches how the repo is formatted, so there was never going to be a problem.

The Unit Test failure isn't the Kontur timeouts either. It fails well before the tests, during collection:

src/config.py:195: in <module>
    ALLOW_BIND_ZIP_FILTER = get_bool_env_var(
src/config.py:35: in get_bool_env_var
    return bool(strtobool(str(value)))
E   ValueError: invalid truth value ''

Unit-Test.yml reads ALLOW_BIND_ZIP_FILTER and the other feature flags from repository secrets, and GitHub doesn't pass secrets to pull requests from forks. They arrive as empty strings, strtobool("") rejects them, and both tests/test_API.py and tests/test_app.py error at import. No test in the suite actually ran.

That should affect any PR opened from a fork, not just this one, so it may be worth a look independently of this change. Happy to open a separate issue if that's useful.

The practical consequence here is that CI can't tell you anything about this PR either way. The before/after comparison in the description was run locally, which is why it's there.

This branch has not been deployed

No deployments
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.

1 participant