apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
36.53k stars 14.15k forks source link

Inclusive or exclusive range filters in API #9237

Open mik-laj opened 4 years ago

mik-laj commented 4 years ago

Helllo,

We have the following ranger filter parameters:

Only closed ranged(in interval notation: [start_xxx, end_xxx]) can be fetched using them.

I think, filters representing ranges should use inclusive start values and exclusive end values (half-closed intervals); in interval notation: [start_xxx, end_xxx).

Exclusive end values are preferable for the following reasons:

More information: https://google.aip.dev/145

Alternatively, we can also add LT and GT filters, but this could only complicate situations without much benefit. We would have 4 different parameters for one field. Changing parameter names and changing behavior seems better to me.

Best regards, Kamil Breguła

mik-laj commented 4 years ago

@houqp I haven't started the discussion on the mailing list about this problem yet, but maybe you have some thought?

houqp commented 4 years ago

@mik-laj yes, [start_xxx, end_xxx) is much easier to use than [start_xxx, end_xxx] from a client's point of view. If we don't want to support all 4 comparison operators, then I would love to see us at least switch to GTE and LT.

potiuk commented 4 years ago

Yep. Agree using GTE and LT is a better approach. I think it also encourages good patterns.

However I think this is only valid in case of dates, not durations. For duration i would also like to have <= 200 seconds not only >= 200 seconds (but it also depends on the resolution of duration).

So I'd opt for all 4 operators for duration but only GTE/LT for dates.

Bowrna commented 1 year ago

I am going through this issue and I have some doubts about it. @mik-laj why do you say only closed range can be fetched using those params? I couldn't understand the difference between a closed range and half closed range filter. Could you explain to me with an example? I will see if I can be helpful in fixing this issue.

Bowrna commented 1 year ago

Any update on the question that I have posted? Gentle reminder..

potiuk commented 1 year ago

I think we should simply add GT and LT filters everywhere. It is simplest and let the user choose what filter they want.

Bowrna commented 1 year ago

ok @potiuk am i right to assume that this issue is about removing LTE and GTE and keeping LT and GT filters alone?

potiuk commented 1 year ago

I think we should keep both.

uranusjr commented 2 months ago

This shouldn’t be too difficult. Just add GT and LT variants of the GTE/LTE filters listed above, and add them as parameters that make sense in the API spec (openapi/v1.yaml). Feel free to contribute if you are inclined to.