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

add DO_NOT_TRACK environment variable support #5000

Merged
merged 5 commits into from
Apr 7, 2022
Merged

add DO_NOT_TRACK environment variable support #5000

merged 5 commits into from
Apr 7, 2022

Conversation

agoblet
Copy link
Contributor

@agoblet agoblet commented Apr 6, 2022 •
edited
Loading

resolves #3540

Description

To be compliant with the Console Do Not Track initiative, this PR adds support for the DO_NOT_TRACK environment variable. DO_NOT_TRACK is a generic version of the dbt-specific SEND_ANONYMOUS_USAGE_STATS variable. Tracking will be disabled if any of these 2 variables specifies that the user does not wish to be tracked.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have added information about my change to be included in the CHANGELOG.

@agoblet agoblet requested a review from a team as a code owner April 6, 2022 06:02
@agoblet agoblet requested review from emmyoop and stu-k April 6, 2022 06:02
@cla-bot cla-bot bot added the cla:yes label Apr 6, 2022
gshank
gshank requested changes Apr 7, 2022
View reviewed changes
Copy link
Contributor

@gshank gshank left a comment

Choose a reason for hiding this comment

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

I don't think that we want two separate flags, SEND_ANONYMOUS_USAGE_STATS and DO_NOT_TRACK. We only want one flag, SEND_ANONYMOUS_USAGE_STATS, but one that can also be set from a DO_NOT_TRACK env var.

@agoblet
Copy link
Contributor Author

agoblet commented Apr 7, 2022

I don't think that we want two separate flags, SEND_ANONYMOUS_USAGE_STATS and DO_NOT_TRACK. We only want one flag, SEND_ANONYMOUS_USAGE_STATS, but one that can also be set from a DO_NOT_TRACK env var.

Thanks for the review @gshank. Having 2 separate flags for the same feature adds complexity to the feature itself. I changed the code to your proposed solution. the DO_NOT_TRACK will override SEND_ANONYMOUS_USAGE_STATS when set. This makes the flags module a bit more complex, since we did not have to deal with overriding flags before. It keeps the tracking module clean though.

@gshank
Copy link
Contributor

gshank commented Apr 7, 2022

Yeah, I realized it would actually be a bit more complicated. But you've done a very nice job of refactoring those bits :) And it does feel better to not have both flags.

@gshank gshank merged commit 58bd750 into dbt-labs:main Apr 7, 2022
@jtcohen6 jtcohen6 mentioned this pull request Apr 12, 2022
Merged
6 tasks
agoblet added a commit to BigDataRepublic/dbt-core that referenced this pull request May 20, 2022
@agoblet
add DO_NOT_TRACK environment variable support (dbt-labs#5000)
b4b16cc
@dbeatty10 dbeatty10 mentioned this pull request Jan 5, 2024
Merged
1 task
mirnawong1 added a commit to dbt-labs/docs.getdbt.com that referenced this pull request Jan 8, 2024
@mirnawong1
DO_NOT_TRACK=0 is not the same as `DBT_SEND_ANONYMOUS_USAGE_STATS=T…
0c740b6 
…rue` (#4713)

## What are you changing in this pull request and why?

While responding to dbt-labs/dbt-core#9336, I
set `DO_NOT_TRACK=0` and examined the result. It does not behave the
same way as `DBT_SEND_ANONYMOUS_USAGE_STATS=True`.

Looking at the source code
[here](https://github.com/dbt-labs/dbt-core/blob/11cc71b75fa64b09888461339eb1eb3b394f9528/core/dbt/cli/flags.py#L252-L254)
explains why.

So we can safely just remove this line from the docs for
[`send_anonymous_usage_stats`](https://docs.getdbt.com/reference/global-configs/usage-stats)
to avoid confusion.

## Additional info

We first added support for the [Console Do Not
Track](https://consoledonottrack.com/) initiative within
dbt-labs/dbt-core#5000 as described in
dbt-labs/dbt-core#3540.

Any of the following are equivalent to
`DBT_SEND_ANONYMOUS_USAGE_STATS=False` (whether they are uppercase,
lowercase, or mixed case):
```
export DO_NOT_TRACK=1
export DO_NOT_TRACK=t
export DO_NOT_TRACK=true
export DO_NOT_TRACK=y
export DO_NOT_TRACK=yes
```

Any other values of `DO_NOT_TRACK` are ignored altogether and not have
any effect.

## Checklist
- [x] Review the [Content style
guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md)
so my content adheres to these guidelines.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gshank gshank gshank approved these changes

@emmyoop emmyoop Awaiting requested review from emmyoop emmyoop was automatically assigned from dbt-labs/core-group

@stu-k stu-k Awaiting requested review from stu-k stu-k was automatically assigned from dbt-labs/core-group

Assignees
No one assigned
Labels
cla:yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support console env var DO_NOT_TRACK
3 participants
@agoblet @gshank @jtcohen6