| dbarratt added a comment. |
In T173214#4736866, @Tpt wrote:I just had a quick review of the current GraphQL structure for Wikibase entities. It looks great! Thank you!
No problem. I'm learning a lot about GraphQL (and the Action API) so it's been fun. :)
- I would switch the Entity type to an interface and have Item, Property, Lexeme... implementations
I went back and forth on this a whole lot. I started out with it just as you described, and I abandoned it. At a certain level, I completely agree this is the way it should be. The entities are different types, so GraphQL should expose it the same way. On the other side of this... it is a dark rabbit hole of despair. We have different types of things all over the place. We have different types of pages, we have different types of images, we have different type of blocks, we even have different types of sites (some have Wikibase, some do not).
Even within entity, it's not clear. You've listed the types of entities, but those are specific to Wikidata. Commons for instance will have a Media type which is M. Should we include that?
Eventually I came to this "rule of the road"
Nullable fields do not justify new types.
What I mean by this, is that if the only difference between a two types is that one has a field and one does not, that does not mean that they should become new types. Actually avoiding new types seems to be way easier (from a development side).
However, I realize that this is probably not ideal. Indeed, the typing can be used for conditions that I hadn't really thought about. For instance, MediaWiki has different image types DRAWING and BITMAP are examples. I implemented the method to get thumbnail images, but without the typing, it makes requests for thumbnail images even if you don't want/need them (for instance, on DRAWING).
I also noticed that the Action API uses a lot of enums for values... should the server maintain a list of these values?
Having robust typing, that is aware of the domain knowledge of the wiki (which is completely configurable), seems like way outside of the scope of the server. For one thing, half of things you'd need to know (like which types of entities exist on a wiki and which fields they have) do not even have API endpoints that you can lookup this information, it only exists in code or in configuration.
With all of that... I think GraphQL requests should be delegated to their individual sites T209133. This way each site runs it's own GraphQL server (as a MediaWiki extension) and can be aware of it's own extensions, and allow each extension to implement it's own typings. That way, the decisions of what types should exist or not exist is delegated to the extensions themselves rather than trying to make that decision for everyone.
- type, datatype and snaktype fields values should probably be enumerations to be more GraphQLish
I'm not sure what that means? I'm just returning what comes back from the Action API, but yeah if we implemented different types that field would no longer be needed.
- I would merge the EntityLabel and SnakValueMonolingualTextValue types because they represent the same object according to the Wikibase DataModel.
That's a great idea. T209439
- I would remove the SnakValue* types (SnakValueString, SnakValueEntity...) because they only provide the "type" field that seems not needed and the feature it covers is already done by the __type introspection field
They actually implement a value key:
type SnakValueString implements SnakValue {
value: String
# SnakValue
type: String
}
type SnakValueEntity implements SnakValue {
value: Entity
# SnakValue
type: String
}This is actually one of the reasons I'm not a huge fan of having multiple types, it's not obvious that you need to specify the types. I had the same problem in T173214#4442448 haha. :)
I would do something like this:
type SnakValue { value: Entity | String type: String }but GraphQL does not allow a union on a scalar. :( and I wanted it to be as close to the existing Action API as possible.
Ironically, this is the place where I did implement different types (out of necessity) and both of us found it more difficult to use. :/ I can't imagine how hard it would be to use if there were different types everywhere, but then again if you expect it maybe it's not that bad?
- I would rename the Claim type to Statement. In the Wikibase DataModel a "claim" is an affirmation (i.e. a main snak and some qualifiers) and the (claim, references, rank) structure is a "statement"
I decided that claims was more apporiate for two reasons.
- Wikibase uses claims in it's API response and I wanted to stay as close to that as possible. https://www.wikidata.org/w/api.php?action="">
- Wikibase exposes a getclaims action to get the claims: https://www.wikidata.org/w/api.php?action="">
From the API response, it actually appears that a Statement is a type of Claim. So really a Claim should be an interface, but the field should remain claims because I assume that Wikibase would allow some other form of claims (that perhaps Wikidata does not use?)
Regardless, this is probably further proof that the typings (or lack there of) should be handled by #mediawiki-extensions-wikibaserepository. I don't really want to be the one making this decision, I would much rather defer it to that extension.
- I would make Snakan interface and have implementations for the different snak types
I don't disagree. I noticed that's how you had it before, but it violated my rule so I didn't implement it, but again, I think that it should be in the extension rather than a separate service. So perhaps T209133 is more important than I originally thought (even though, it might be an uphill battle to get it onto production and enabled on every wiki).
Cc: Daniel_Mietchen, gerritbot, Smalyshev, Lydia_Pintscher, Addshore, larsgw, Saerdnaer, simon04, bearND, Siznax, Tpt, Jonas, Ricordisamoa, hoo, Lucas_Werkmeister_WMDE, Aklapper, dbarratt, PokestarFan, CucyNoiD, Nandana, NebulousIris, Gaboe420, Versusxo, Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Lahi, Gq86, Baloch007, Darkminds3113, Bsandipan, Lordiis, GoranSMilovanovic, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, QZanden, LawExplorer, Lewizho99, Maathavan, D3r1ck01, Wikidata-bugs, aude, He7d3r, Jdforrester-WMF, Mbch331, Jay8g, Tgr
_______________________________________________ Wikidata-bugs mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
