-
Notifications
You must be signed in to change notification settings - Fork 903
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
Add an option for kedro new to skip telemetry #3701
Changes from all commits
90fcd26
8d0d13b
a8b0036
482d093
6504100
09f174b
8917e85
2b3b046
583e5e7
f8254d0
fb5a876
c2c3906
650f37c
19ac1e7
ecc3780
329b05f
31089ee
39b9474
8733b2a
6c4ce42
c11f197
b34539c
3ee92fa
ef827ef
575929d
603c894
f9bcd70
29184db
51265e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1571,3 +1571,119 @@ def test_convert_tool_short_names_to_numbers_with_duplicates(self): | |
selected_tools = "lint,test,tests" | ||
result = _convert_tool_short_names_to_numbers(selected_tools) | ||
assert result == ["1", "2"] | ||
|
||
|
||
@pytest.mark.usefixtures("chdir_to_tmp") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moving the tests related to tools to their own file and maybe moving auxiliary functions to their own file as well would probably solve half ot it. |
||
class TestTelemetryCLIFlag: | ||
@pytest.mark.parametrize( | ||
"input", | ||
["yes", "YES", "y", "Y", "yEs"], | ||
) | ||
def test_flag_value_is_yes(self, fake_kedro_cli, input): | ||
result = CliRunner().invoke( | ||
fake_kedro_cli, | ||
[ | ||
"new", | ||
"--name", | ||
"New Kedro Project", | ||
"--tools", | ||
"none", | ||
"--example", | ||
"no", | ||
"--telemetry", | ||
input, | ||
], | ||
) | ||
|
||
repo_name = "new-kedro-project" | ||
assert result.exit_code == 0 | ||
|
||
telemetry_file_path = Path(repo_name + "/.telemetry") | ||
assert telemetry_file_path.exists() | ||
|
||
with open(telemetry_file_path) as file: | ||
file_content = file.read() | ||
target_string = "consent: true" | ||
|
||
assert target_string in file_content | ||
_clean_up_project(Path("./" + repo_name)) | ||
|
||
@pytest.mark.parametrize( | ||
"input", | ||
["no", "NO", "n", "N", "No"], | ||
) | ||
def test_flag_value_is_no(self, fake_kedro_cli, input): | ||
result = CliRunner().invoke( | ||
fake_kedro_cli, | ||
[ | ||
"new", | ||
"--name", | ||
"New Kedro Project", | ||
"--tools", | ||
"none", | ||
"--example", | ||
"no", | ||
"--telemetry", | ||
input, | ||
], | ||
) | ||
|
||
repo_name = "new-kedro-project" | ||
assert result.exit_code == 0 | ||
|
||
telemetry_file_path = Path(repo_name + "/.telemetry") | ||
assert telemetry_file_path.exists() | ||
|
||
with open(telemetry_file_path) as file: | ||
file_content = file.read() | ||
target_string = "consent: false" | ||
|
||
assert target_string in file_content | ||
_clean_up_project(Path("./" + repo_name)) | ||
|
||
def test_flag_value_is_invalid(self, fake_kedro_cli): | ||
result = CliRunner().invoke( | ||
fake_kedro_cli, | ||
[ | ||
"new", | ||
"--name", | ||
"New Kedro Project", | ||
"--tools", | ||
"none", | ||
"--example", | ||
"no", | ||
"--telemetry", | ||
"wrong", | ||
], | ||
) | ||
|
||
repo_name = "new-kedro-project" | ||
assert result.exit_code == 1 | ||
|
||
assert ( | ||
"'wrong' is an invalid value for example pipeline. It must contain only y, n, YES, or NO (case insensitive)." | ||
in result.output | ||
) | ||
|
||
telemetry_file_path = Path(repo_name + "/.telemetry") | ||
assert not telemetry_file_path.exists() | ||
|
||
def test_flag_is_absent(self, fake_kedro_cli): | ||
result = CliRunner().invoke( | ||
fake_kedro_cli, | ||
[ | ||
"new", | ||
"--name", | ||
"New Kedro Project", | ||
"--tools", | ||
"none", | ||
"--example", | ||
"no", | ||
], | ||
) | ||
|
||
repo_name = "new-kedro-project" | ||
assert result.exit_code == 0 | ||
|
||
telemetry_file_path = Path(repo_name + "/.telemetry") | ||
assert not telemetry_file_path.exists() |
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.
I will take some reference from https://github.com/kedro-org/kedro-plugins/tree/main/kedro-telemetry, but probably get advice from @astrojuanlu since we need to be careful about the wordings. We did collect some Project information, such as number of pipelines, datasets, but they are anonymous.
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.
That's a good point, I will let him know.
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.
I would copy the definition from the regex function as it's more descriptive:
It must contain only y, n, YES, or NO (case insensitive).