dstandish commented on code in PR #26942:
URL: https://github.com/apache/airflow/pull/26942#discussion_r990717913
##########
airflow/www/views.py:
##########
@@ -3574,13 +3600,32 @@ def datasets_summary(self):
DatasetModel.id,
DatasetModel.uri,
)
- .filter(DatasetModel.uri.ilike(f"%{uri_pattern}%"))
.order_by(*order_by)
- .offset(offset)
- .limit(limit)
- .all()
- ]
- data = {"datasets": datasets, "total_entries": total_entries}
+ )
+
+ if uri_pattern:
+ count_query =
count_query.filter(DatasetModel.uri.ilike(f"%{uri_pattern}%"))
+ query =
query.filter(DatasetModel.uri.ilike(f"%{uri_pattern}%"))
+
+ if updated_after:
+ count_query = count_query.outerjoin(
Review Comment:
adding these filters presents an ambiguity.
do you still return all the datasets, but the filters only apply to the
counts?
or, do you just return the datasets with updates in the time range.
personally i think the more intuitive behavior is to return only those
datasets with updates in the period. but, i guess this is just an internal API
so really we have to look at why we're adding this and how it's gonna be used.
so in short, are you sure you want a left join here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
