Support collections editing and new DS type

[blokhin]

@knopki client.v0.collections.create() also supports editing the existing collections (the id has to be supplied), see https://github.com/basf/metis-bff/blob/f6f73257d6f2603496118676969663e4d6e4deaa/routes/%5Bversion%5D/collections/_helpers.js#L65

However here is a caveat: 3 kwargs typeId, dataSources, and users are mapped onto type_id, data_source_ids, and user_ids (note camel case vs. snake case) which prevents me from doing client.v0.collections.create(**dict(metis_collection_changed)). I assume, the MetisCollectionCreateDTO should be upgraded for that, right?

[blokhin]

TODO setting id=None for creating a new collection

[pep8speaks]

Hello @blokhin! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-01-12 20:05:28 UTC

[knopki]

Overall, it's good PR. But fix style, please.

[knopki]

Why docstring remove?

[blokhin]

because it doesn’t bring any valuable information here

[knopki]

Correct. So there are two ways to go about it: write valuable docstring or turn off the linter rule missing-class-docstring.

[knopki]

However here is a caveat: 3 kwargs typeId, dataSources, and users are mapped onto type_id, data_source_ids, and user_ids (note camel case vs. snake case) which prevents me from doing client.v0.collections.create(**dict(metis_collection_changed)). I assume, the MetisCollectionCreateDTO should be upgraded for that, right?

Or MetisCollectionsCreateKwargs and create_event(...)? Snake case looks like a bug.

[blokhin]

The different case originates from the BFF API, which being in JavaScript contradicts Python style. How could we map fields nicely onto each other so that one doesn’t have to copy them manually? Maybe to implement constructor for the typed dict?

[knopki]

BFF API already mapped (manually) to Python-style DTOs in client code. Why copy manually?

Maybe to implement constructor for the typed dict?

TypedDict classes can contain only type annotation. TypedDict is a thin type. If you need a constructor or an additional method of a class or instance, it is better to use dataclass.

[blokhin]

BFF API already mapped (manually) to Python-style DTOs in client code. Why copy manually?

Nope. Otherwise as said I would be able to do client.v0.collections.create(**dict(metis_collection_changed)) but now it’s impossible. So I have to do metis_collection_changed["type_id"] = metis_collection_changed["typeId"] etc.

[blokhin]

@knopki how do I better implement client.v0.collections.create(**dict(metis_collection_changed)) ?

[knopki]

@knopki how do I better implement client.v0.collections.create(**dict(metis_collection_changed)) ?

I recommend correcting the stylistic errors in this PR (black, pylint) and merge it. Then discuss the problem with dictionaries, as it can be quite large.