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

Search for Darwin when Mac is selected #2818

Merged
merged 1 commit into from
May 14, 2024
Merged

Conversation

edugfilho
Copy link
Collaborator

Found this while testing other stuff. GLAM erroneously reporting no data for Mac metric all along because in the data it's called Darwin.

@edugfilho edugfilho requested a review from Iinh May 14, 2024 19:28
Comment on lines 43 to 46
{ key: '*', label: 'All OSes' },
{ key: 'Windows', label: 'Windows' },
{ key: 'Mac', label: 'Mac' },
{ key: 'Darwin', label: 'Mac' },
{ key: 'Linux', label: 'Linux' },
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at this ingestion code, I think we're missing more than just Mac?

Copy link
Collaborator Author

@edugfilho edugfilho May 14, 2024

Choose a reason for hiding this comment

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

Interesting. I did find a very small amount of BSD in the data. The thing is if we want to accommodate more than one name into a single alias, we have to do that at the ETL level, otherwise the aggregations won't make sense. I say we land this so ppl can look at Mac data and open an issue for your find?

Copy link
Contributor

@Iinh Iinh May 14, 2024

Choose a reason for hiding this comment

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

I agree. Maybe good to use this normalized os UDF at the ETL level - mozfun.norm.os(normalized_os).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perfect! Thanks for finding this. Would you mind opening the ticket with this precious info?

Copy link
Contributor

Choose a reason for hiding this comment

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

done!

#2819

@edugfilho edugfilho merged commit 68b024c into main May 14, 2024
4 checks passed
@edugfilho edugfilho deleted the mac-is-called-darwin branch May 14, 2024 19:45
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.

2 participants