search

bakdata / kpops

Deploy Kafka pipelines to Kubernetes
https://bakdata.github.io/kpops
MIT License
12 stars 1 forks source link

Migrate to Pydantic v2 #347

Closed sujuka99 closed 9 months ago

sujuka99 commented 1 year ago

closes #341

sujuka99 commented 11 months ago

We have a problem with Environment variable names that differ from the field name.. See here

A good example is the brokers field in PipelineConfig

We could do the renaming of config settings in this PR? I could maybe write a new SettingsSrouce to work around the bug, but I'm not sure if it's worth it.

disrupted commented 10 months ago

A good example is the brokers field in PipelineConfig

this is already renamed in v3. does that solve it?

sujuka99 commented 10 months ago

A good example is the brokers field in PipelineConfig

this is already renamed in v3. does that solve it?

Yes, if we have no fields different in name from their respective env vars, we wouldn't be affected by the bug.

sujuka99 commented 10 months ago

@disrupted Is adding titles to the models relevant to the schema generation?

disrupted commented 10 months ago

Closes #20 ?

sujuka99 commented 10 months ago

Closes #20 ?

Not if we want full support (loading all env vars from the file)

I believe Pydantic doesn't export the variables in the dotenv file to os.env, but instead directly reads only the settings from it.

Relevant issue

I am pretty sure that adding the support for everything else wouldn't be hard, but I would prefer to do it in another PR.

sujuka99 commented 10 months ago

credit: @disrupted

Problems found during testing on real projects:

  1. app.streams.config is forced to have string values now, e.g. max.poll.records: 100 is not possible anymore (Must be fixed now)
  2. Dry-run error: RuntimeError: Error: release name is invalid: wordwordwo-word-wordwordwordw-wordw-wordwor-wordwordwo. That release name should be automatically trimmed if it's over the character limit.