Closed fsteeg closed 6 months ago
Thanks a lot for your review @tfmorris! I replied inline to your specific comments.
It feels like this spec is accumulating more and more words/features without a commensurate increase in real world testing experience (unless there's implementation testing going on behind the scene that I'm missing). But it's all just intuition / gut feeling...
I got to say I do share that gut feeling! What I actually tried to do here was to condense all these different suggestions from the 5 linked issues into a fairly compact, non-breaking proposal (optional fields and retaining the current behavior if omitted) that addresses actual use cases.
There is one use case I forgot to mention: https://github.com/OpenRefine/OpenRefine/issues/5615 could be implemented using required
properties with match_quantifiers
instead of the type filter.
One of the things that I like about the IETF process is that they require prior implementation and testing. This demonstrates that a) it's implementable and b) there's at least one real world use case.
Yes, I agree, and it's what I like so much about working on this spec, that it is based on years of proven functionality, and not "made up". I very much want to keep it that way too! After we addressed any obvious issues with the approach proposed in this PR, we should implement it in the testbench and our service to see if it actually works.
No matter what syntax, we need to keep the ability for users to always optionally provide constraints (filters and property matching criteria) at query time, and not depend on services to have some fancy logic to figure things out the clients wants or wishes to return an adequate response. So I always prefer syntax that keeps more control for users.
This adds three optional fields to properties in reconciliation queries. It's based on our discussions in recent meetings, in particular the suggestion in https://github.com/reconciliation-api/specs/issues/105#issuecomment-1463754747, adding an additional field to address #114, based on https://github.com/reconciliation-api/specs/issues/128#issuecomment-1592778805.
Changes from the linked discussions:
mode=must|should
withrequired=true|false
(we discussedfilter
as an option in our last meeting, but I feltrequired
was clearer since filter can be used both meaning "accept" and "reject")match=any|all|none
tomatch_quantifier=any|all|none
match_qualifier=<string>
to address use cases like #114, which came up in otherwise unrelated https://github.com/reconciliation-api/specs/issues/128#issuecomment-1592778805This is related to multiple issues, which I'd hope this could resolve:
match_quantifier
required
match_quantifier
)match_qualifier
and the mention of the EDTF specificationquery
is optional already, but with this we have precise control over the behavior of the property replacing thequery