-
Notifications
You must be signed in to change notification settings - Fork 84
fix: add opt-out flag to fides init #6640
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
base: main
Are you sure you want to change the base?
fix: add opt-out flag to fides init #6640
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Someone is attempting to deploy a commit to the Ethyca Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR adds a --opt-out flag to the fides init command, enabling non-interactive setups to disable analytics collection. The implementation includes proper validation to prevent conflicting flags (--opt-in and --opt-out cannot be used together) and threads the new parameter through the configuration creation process.
Key changes include:
- Added
--opt-outCLI flag with help text and validation logic - Updated
request_analytics_consent()to handle bothopt_inandopt_outparameters - Enhanced parameter threading in
create_and_update_config_file() - Added comprehensive test coverage for all new functionality and edge cases
- Updated changelog entry to document the new feature
Confidence Score: 4/5
- This PR is safe to merge with minimal risk
- The implementation is straightforward with proper validation, comprehensive test coverage, and follows existing patterns. No security concerns or breaking changes identified.
- No files require special attention - all changes are well-implemented and tested
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| src/fides/cli/commands/ungrouped.py | 4/5 | Added --opt-out flag to fides init command with proper validation against conflicting --opt-in flag |
| src/fides/cli/utils.py | 3/5 | Updated analytics consent logic to handle both opt-in and opt-out parameters with validation |
| tests/ctl/core/config/test_create.py | 4/5 | Added comprehensive test coverage for new opt-out flag functionality and edge cases |
Sequence Diagram
sequenceDiagram
participant User
participant CLI as fides init
participant Validation as Flag Validation
participant Utils as request_analytics_consent
participant Config as FidesConfig
participant EnvVar as Environment
User->>CLI: fides init --opt-out
CLI->>Validation: Check opt_in and opt_out flags
alt Both flags provided
Validation->>User: Error: cannot use both flags
else Only opt-out provided
CLI->>Utils: request_analytics_consent(opt_out=True)
Utils->>EnvVar: Check FIDES__USER__ANALYTICS_OPT_OUT
alt Environment variable set to true
EnvVar->>Config: Set analytics_opt_out = True
Config->>CLI: Return config
else No environment override
Utils->>Config: Set analytics_opt_out = True
Config->>CLI: Return config (no prompt)
end
else Only opt-in provided
CLI->>Utils: request_analytics_consent(opt_in=True)
Utils->>Config: Set analytics_opt_out = False
Config->>CLI: Return config (no prompt)
else No flags provided
CLI->>Utils: request_analytics_consent()
Utils->>User: Prompt for consent
User->>Utils: Response
Utils->>Config: Set analytics_opt_out accordingly
Config->>CLI: Return config
end
CLI->>User: Continue initialization
5 files reviewed, 1 comment
| if opt_in: | ||
| config.user.analytics_opt_out = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Logic error: when opt_in=True, analytics_opt_out should be False, not True.
| if opt_in: | |
| config.user.analytics_opt_out = False | |
| if opt_in: | |
| config.user.analytics_opt_out = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch—this reflects the intended behavior. Applied your suggestion in 6bd3a08.
Summary
fides initdoes not have a--opt-outflag #4703--opt-outflag tofides initso non-interactive setups can disable analytics--opt-in/--opt-outand thread the new option through config creationTesting
Manual Verification
example.envto.envif needed.nox -s dev -- shell. Once the shell opens, run:mkdir -p /tmp/fides-init-check && cd /tmp/fides-init-checkfides init --opt-outgrep analytics_opt_out .fides/fides.toml(should readtrue)fides init --opt-in --opt-out(fails with "--opt-in and --opt-out cannot be used together")exitto leave the container shell.nox -s pytest -- ctl-unit. (This coverstests/ctl/core/config/test_create.py; alternatively run the file directly inside the container if preferred.)nox -s teardown -- volumes.