WCM-285: revert IConnector <-> DAVPropertyConverter behaviour #857
Conversation
stollero
changed the title
core/src/zeit/connector/models.py
Outdated
|
|
||
| unsorted = collections.defaultdict(dict) | ||
| for (k, ns), v in props.items(): | ||
| if v is DeleteProperty: | ||
| continue | ||
| if ns == INTERNAL_PROPERTY: | ||
| continue | ||
| if isinstance(v, datetime): |
There was a problem hiding this comment.
Wir können in unsorted natürlich nur JSON serializable objects schreiben. datetime bspw. funktioniert nicht.
Ich überlege, welche sinnvolle Lösungsmöglichkeiten es gibt (unabhängig von dem Ansatz, den ich hier ausgebaut habe).
There was a problem hiding this comment.
Ich glaube, die Lage ist sogar noch etwas anders: Im unsorted stehen bisher nur Strings drin, weil wir die Werte 1:1 aus dem DAV dorthin migriert haben, und dort waren nur Strings möglich. Und die IDAVPropertyConverter gehen auch davon aus, dass da nur Strings drinstehen. Wir fangen jetzt zwar mit den "MetadataColumns" an, da ausnahmen zuzulassen, aber da geht es ja nur um Felder, die nicht in unsorted stehen.
Der langen Rede kurzer Sinn: Wenn in IResource.properties ein non-String Wert drinsteht, der weder als internal deklariert noch in seine eigene Spalte umgezogen ist, so ist das ein Programmierfehler. Den sollten wir denke ich also definitiv nicht verschlucken (continue) -- ich würde aber sagen, dazu tun wir hier (nach wie vor) einfach gar nichts, sondern das kann dann gern "weiter hinten" als "not json serializable" knallen.
wosc
changed the title
core/src/zeit/connector/models.py
Outdated
|
|
||
| unsorted = collections.defaultdict(dict) | ||
| for (k, ns), v in props.items(): | ||
| if v is DeleteProperty: | ||
| continue | ||
| if ns == INTERNAL_PROPERTY: | ||
| continue | ||
| if isinstance(v, datetime): |
There was a problem hiding this comment.
Ich glaube, die Lage ist sogar noch etwas anders: Im unsorted stehen bisher nur Strings drin, weil wir die Werte 1:1 aus dem DAV dorthin migriert haben, und dort waren nur Strings möglich. Und die IDAVPropertyConverter gehen auch davon aus, dass da nur Strings drinstehen. Wir fangen jetzt zwar mit den "MetadataColumns" an, da ausnahmen zuzulassen, aber da geht es ja nur um Felder, die nicht in unsorted stehen.
Der langen Rede kurzer Sinn: Wenn in IResource.properties ein non-String Wert drinsteht, der weder als internal deklariert noch in seine eigene Spalte umgezogen ist, so ist das ein Programmierfehler. Den sollten wir denke ich also definitiv nicht verschlucken (continue) -- ich würde aber sagen, dazu tun wir hier (nach wie vor) einfach gar nichts, sondern das kann dann gern "weiter hinten" als "not json serializable" knallen.
| ) | ||
|
|
||
| def test_converts_aggregate_types_on_read(self): | ||
| example_channels = {'channel1': [], 'channel2': ['sub1', 'sub2']} |
There was a problem hiding this comment.
Wir wollen die Channels in der DB genau wie in Python als nested Liste speichern ([["channel1", None], ["channel2", "sub1"], ["channel2", "sub2"]]), war mein Kenntnisstand. Einen Usecase für ein dict haben wir aktuell glaub ich noch gar nicht (könnte aber grundsätzlich kommen, z.B. bei Image Variants oder sowas).
|
|
||
|
|
||
| @grok.implementer(IConverter) | ||
| class DictConverter(grok.Adapter): |
There was a problem hiding this comment.
Die Aufgabe dieser Converter ist, mal platt gesagt, vorhandene .meta Files in den zeit.web Testdaten zu parsen, um die Datentypen draus zu machen, die sowieso rauskämen, wenn es SQL wäre. Dh konkret gesprochen brauchen wir hier einen, der für Channels aus "chan1;chan2 sub1;chan2 sub2" dann [['chan1', None], ['chan2', 'sub1'], ['chan2', 'sub2']] macht, und umgekehrt (dafür könnte man vmtl den CollectionTextLineProperty/ChannelProperty Code aufrufen, um die Implementierung nicht duplizieren zu müssen).
There was a problem hiding this comment.
Die nächste Frage ist dann, wie man den passenden Converter findet, weil es jetzt ja losgeht, dass wir SQL-seitig sagen "jo klar, ist halt json", aber je nach dem konkreten Feld in dem json dann Dict oder Liste oder sonstwas drinsteht. Das heißt aber, dass es nicht mehr ausreicht, den IConverter rein anhand des Column-Typs nachzuschlagen. Insofern würde ich vorschlagen, dass wird den "Haupt-Adapter" (derzeit converter_from_column_type) umbenennen und erweitern, und im ersten Schritt schauen, ob es einen named Adapter für genau diesen namespace/name gibt (darauf könnte man dann document/channels hängen), und erst im zweiten Schritt anhand des Typs schauen.
Checklist
gif