Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command, command options and test bot data #437

Merged
merged 1 commit into from Apr 26, 2024

Conversation

MikeMcQuaid
Copy link
Member

These new categories are available from Homebrew/brew#16847

There's a few TODO here that I'd love help with but don't need to block getting this data flowing.

@MikeMcQuaid MikeMcQuaid marked this pull request as draft April 26, 2024 08:36
@MikeMcQuaid MikeMcQuaid force-pushed the analytics_command_run_test_bot branch 4 times, most recently from 16c8fcb to dba4309 Compare April 26, 2024 14:01
These new categories are available from
Homebrew/brew#16847

There's a few TODO here that I'd love help with but don't need to
block getting this data flowing.
@MikeMcQuaid MikeMcQuaid force-pushed the analytics_command_run_test_bot branch from dba4309 to 788e815 Compare April 26, 2024 14:26
@MikeMcQuaid MikeMcQuaid marked this pull request as ready for review April 26, 2024 14:45
@MikeMcQuaid MikeMcQuaid merged commit d3315a8 into master Apr 26, 2024
2 checks passed
@MikeMcQuaid MikeMcQuaid deleted the analytics_command_run_test_bot branch April 26, 2024 14:45
Comment on lines +227 to +234
# Cleanup bad data before https://github.com/Homebrew/homebrew-test-bot/pull/1043
# TODO: actually delete this from InfluxDB.
# Can delete this code after 27th April 2025.
next if %w[audit install linkage style test].exclude?(command_and_package.first)
next if command_and_package.last.include?("/")
next if options.include?("--tap=")
next if options.include?("--only-dependencies")
next if options.include?("--cached")
Copy link
Member Author

Choose a reason for hiding this comment

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

@Bo98 @SMillerDev could do with some help figuring out if we can just delete the data here. I tried and failed:

$ influx delete --bucket analytics --predicate '_measurement="test_bot_test"' --start='2024-04-01T00:00:00Z' --stop='2024-04-26T07:07:19.000Z' --org-id *** --host '***' --token="***"

This ran successfully but didn't seem to delete anything 🤷🏻. Help?

Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2024-05-02 at 19 54 08

Copy link
Member

Choose a reason for hiding this comment

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

The CLI as a whole doesn't really work with InfluxDB v3.

Comment on lines +285 to +286
# TODO: we don't seem to get a valid count for these categories, unclear why.
count ||= 1 if count_being_weird_categories.include?(category)
Copy link
Member Author

Choose a reason for hiding this comment

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

@SMillerDev @Bo98 (and any other @homebrew/maintainers who understand InfluxDB): similarly: it'd be good to figure this out. I'm actually not convinced the counts are correct in this case (so am not going to have the data displayed publicly anywhere yet) but hopefully this being merged gives you enough to reproduce the weirdness with some puts or pp.

Copy link
Member

@Bo98 Bo98 May 2, 2024

Choose a reason for hiding this comment

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

InfluxQL does not support grouping by fields and options is a field rather than a tag.

SQL however seems to? Or at least it appears to work fine in the query webpage. Will require using a totally different API to send the query but I can have a look at doing that.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Bo98 Ah, makes sense now, thanks. Yeh, I think moving this to SQL would be desirable on multiple counts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants