Skip to content

Conversation

@sgiehl
Copy link
Member

@sgiehl sgiehl commented Nov 17, 2025

Description

Checklist

  • [✔] I have understood, reviewed, and tested all AI outputs before use
  • [✔] All AI instructions respect security, IP, and privacy rules

Review

@sgiehl sgiehl force-pushed the dev-19593 branch 2 times, most recently from 1c2ed65 to a3746a7 Compare November 17, 2025 16:47
@sgiehl sgiehl force-pushed the dev-19555 branch 2 times, most recently from e7920ce to f622e04 Compare November 18, 2025 09:24
@sgiehl sgiehl force-pushed the dev-19593 branch 2 times, most recently from 9f6fa80 to 833aceb Compare November 18, 2025 12:57
@sgiehl sgiehl marked this pull request as ready for review November 18, 2025 12:58
@sgiehl sgiehl requested a review from a team November 19, 2025 07:57
@sgiehl sgiehl force-pushed the dev-19555 branch 3 times, most recently from 19a3fe9 to 785cf6d Compare November 20, 2025 11:10
@sgiehl sgiehl force-pushed the dev-19593 branch 2 times, most recently from 2e69688 to 5ba49b7 Compare November 20, 2025 14:14
@sgiehl sgiehl added this to the 5.7.0 milestone Nov 24, 2025
Base automatically changed from dev-19555 to 5.x-dev November 25, 2025 19:34
@sgiehl sgiehl force-pushed the dev-19593 branch 4 times, most recently from 0ec9857 to a53140a Compare November 28, 2025 12:52
$bindBase = $logAggregator->getGeneralQueryBindParams();

$sql = sprintf(
"SELECT * FROM (SELECT bot.bot_name, log_action.name AS url, COUNT(*) AS requests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a ranking style query with the usual Others limitation here?

Currently,y this is completely unbound and just fetches everything, to be later truncated based on the configuration values.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually was thinking about that in the first place, but didn't want to add too much complexity. But I guess as soon as someone, tracking millions of different page urls or downloads, tries to start using this it can cause trouble 🤔
Shall we create a follow up issue to add that later, so we can deliver this one faster?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up sounds great 👍

@sgiehl sgiehl force-pushed the dev-19593 branch 2 times, most recently from 273e682 to 570a609 Compare December 2, 2025 13:56
@sgiehl sgiehl requested a review from mneudert December 2, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants