search

openeduhub / metaqs-main

Backend Service providing Information about Completeness of Metadata and Coverage of Topics
3 stars 1 forks source link

50 bug wrong response type for glom #51

Closed RobertMeissner closed 2 years ago

RobertMeissner commented 2 years ago

Due to an unexpected entry in general_keywords for collection "Informatik", this endpoint failed in production and had to be mended. The corresponding endpoint was transfered here.I refactored specificially enum unions which caused their entries to be hidden, the naming of certain elasticsearch code and generalized the usage of glom - the later could be replaced by our own implementation if we want to reduce dependencies.

RobertMeissner commented 2 years ago

Some minor things.

The biggest point (which isn't realls in the scope of this PR) is the following:

Currently there is a huge machinery around building elastic search request and processing the responses.

We have a separation into two different enums (ElasticResourceAttribute, and CollectionAttribute) which seems entirely arbitrary to me. It also seems that the separation into enums has to be aligned with some other internal data structures (the request building and the internal data structures into which the responses will be transformed).

As far as I understood the overall procedure is about the following

  • Issue request to elastic yielding a deeply nested request specific json
  • use the pair of (glom-specs, ...Attribute enum) to transform the response dictionary into the request specific internal representation which would be list[MissingMaterials] or list[CollectionNode]

Currently this whole process is scattered over a quite large amount of different files. It would be nice to have that all together, i.e. request building, response specs, and internal data model definition.

But as stated above, thats definitely out of scope for this PR...

The spreading of code over many files is a valuable observation - I had not bothered yet. This will be part of the further refactoring of elasticsearch for the next endpoints. But removing it completely, i.e., elasticsearch-dsl, I am not a fan of right now. I suggest we have a chat on this when I continue working here, alright?