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.

Sign up for GitHub

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

Jump to bottom

Disable analytics option has wrong behavior #1983

Closed
nicolasvienot opened this issue Dec 8, 2021 · 2 comments · Fixed by #1984
Closed

Disable analytics option has wrong behavior #1983

nicolasvienot opened this issue Dec 8, 2021 · 2 comments · Fixed by #1984
Assignees
@Kerollmops
Labels
bug Something isn't working as expected v0.25.0 PRs/issues solved in v0.25.0
Milestone
v0.25.0

Comments

@nicolasvienot
Copy link
Member

Describe the bug

Following the documentation, we should be able to disable/enable the built-in telemetry by setting --no-analytics or MEILI_NO_ANALYTICS to true/false.

This is not the current behavior of MeiliSearch, which is :

  • If the variable is set (to anything), MeiliSearch built-in telemetry is disabled
  • If the variable is not used, MeiliSearch built-in telemetry is enabled

To Reproduce

Steps to reproduce the behavior:

~ MEILI_NO_ANALYTICS=true ./meilisearch-0.24.0 -> Analytics OFF
~ MEILI_NO_ANALYTICS=false ./meilisearch-0.24.0 -> Analytics OFF
~ MEILI_NO_ANALYTICS="" ./meilisearch-0.24.0 -> Analytics OFF
~ ./meilisearch-0.24.0 --no-analytics true -> Analytics OFF
~ ./meilisearch-0.24.0 --no-analytics false -> Analytics OFF
~ ./meilisearch-0.24.0 --no-analytics "" -> Analytics OFF

~ ./meilisearch-0.24.0 -> Analytics ON

Expected behavior

~ MEILI_NO_ANALYTICS=false ./meilisearch-0.24.0 -> Analytics ON
~ ./meilisearch-0.24.0 --no-analytics false -> Analytics ON

OR Documentation update. @meilisearch/docs-team

MeiliSearch version:

v0.24.0, v0.23.1
@Kerollmops Kerollmops self-assigned this Dec 8, 2021
@Kerollmops Kerollmops mentioned this issue Dec 8, 2021
Merged
@curquiza curquiza added this to the v0.25.0 milestone Dec 8, 2021
@curquiza curquiza added the bug Something isn't working as expected label Dec 8, 2021
@curquiza
Copy link
Member

curquiza commented Dec 8, 2021

Thanks for the detailed issue @nicolasvienot

bors bot added a commit that referenced this issue Dec 8, 2021
@bors @Kerollmops
Merge #1984
879cc4e 
1984: Support boolean for the no-analytics flag r=Kerollmops a=Kerollmops

This PR fixes an issue with the `no-analytics` flag that was ignoring the value passed to it, therefore a `no-analytics false` was just understood as a `no-analytics` and was effectively disabling the analytics instead of enabling them. I found [a closed issue about this exact behavior on the structopt repository](TeXitoi/structopt#468) and applied it here.

I don't think we should update the documentation as it must have worked like this from the start of this project. I tested it on my machine and it is working great now. Thank you `@nicolasvienot` for this issue report.

Fixes #1983.

Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Co-authored-by: Clément Renault <clement@meilisearch.com>
@curquiza
Copy link
Member

curquiza commented Dec 9, 2021

Closed by #1984

@curquiza curquiza closed this as completed Dec 9, 2021
@curquiza curquiza added the v0.25.0 PRs/issues solved in v0.25.0 label Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Kerollmops Kerollmops

Labels
bug Something isn't working as expected v0.25.0 PRs/issues solved in v0.25.0
Projects
None yet
Milestone
v0.25.0
Development

Successfully merging a pull request may close this issue.

Support boolean for the no-analytics flag meilisearch/meilisearch
3 participants
@Kerollmops @curquiza @nicolasvienot