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

fix(telemetry): Update README.md for opt-out model #790

Merged

Conversation

DimedS
Copy link
Contributor

@DimedS DimedS commented Jul 29, 2024

Description

Update the kedro-telemetry README.md following the transition to an opt-out model for telemetry.

Development notes

Checklist

  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the relevant RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Dmitry Sorokin <dmd40in@gmail.com>
@DimedS DimedS linked an issue Jul 29, 2024 that may be closed by this pull request
@DimedS DimedS marked this pull request as ready for review July 29, 2024 15:22
kedro-telemetry/README.md Outdated Show resolved Hide resolved
kedro-telemetry/README.md Show resolved Hide resolved
kedro-telemetry/README.md Outdated Show resolved Hide resolved
```console
pip install kedro-telemetry
```
1. **Set Environment Variables**:
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we also add some explanation on what happens if several options are applied at the same time, how they redefine each other?

Relates to: #785

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch with that bug. I added a description about the priority of the environment variables and overriding the .telemetry file. I believe we will fix the bug.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, let's wait until the fix and then revisit this paragraph to make sure they align.

kedro-telemetry/README.md Outdated Show resolved Hide resolved
DimedS and others added 3 commits July 30, 2024 11:26
Co-authored-by: ElenaKhaustova <157851531+ElenaKhaustova@users.noreply.github.com>
Signed-off-by: Dmitry Sorokin <40151847+DimedS@users.noreply.github.com>
Signed-off-by: Dmitry Sorokin <dmd40in@gmail.com>
Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Added some comments in the other PR. I see the text is very similar, maybe we should focus on one of them first.

Signed-off-by: Dmitry Sorokin <dmd40in@gmail.com>
DimedS and others added 2 commits July 31, 2024 14:39
Signed-off-by: Dmitry Sorokin <dmd40in@gmail.com>
Copy link
Contributor

@ElenaKhaustova ElenaKhaustova left a comment

Choose a reason for hiding this comment

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

Thank you, @DimedS, LGTM!

Let's merge after we agree on #785

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Let's go!

@DimedS DimedS merged commit 84cc29a into main Aug 1, 2024
10 checks passed
@DimedS DimedS deleted the 731-kedro-telemetry-update-telemetry-docs-for-opt-out-model branch August 1, 2024 11:15
merelcht pushed a commit to galenseilis/kedro-plugins that referenced this pull request Aug 27, 2024
* Update README.md after opt-out

---------

Signed-off-by: Dmitry Sorokin <dmd40in@gmail.com>
Signed-off-by: Dmitry Sorokin <40151847+DimedS@users.noreply.github.com>
Co-authored-by: ElenaKhaustova <157851531+ElenaKhaustova@users.noreply.github.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
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.

kedro-telemetry: update telemetry docs for opt-out model
4 participants