diff --git a/src/README.rst b/src/README.rst index c6b9d495..99414121 100644 --- a/src/README.rst +++ b/src/README.rst @@ -16,6 +16,10 @@ To get started, after installation run the following: Change Log ========== +7.0.2 +---------- +- Prompt on first use for telemetry. Update code for 7.0.2 release (#170) + 7.0.1 ---------- - Fix bug where an empty directory is generated in the current location. Update code for 7.0.1 release (#167) diff --git a/src/setup.py b/src/setup.py index ddfcbf9d..98e9599d 100644 --- a/src/setup.py +++ b/src/setup.py @@ -17,7 +17,7 @@ def read(fname): setup( name='sfctl', - version='7.0.1', + version='7.0.2', description='Azure Service Fabric command line', long_description=read('README.rst'), url='https://github.com/Azure/service-fabric-cli', diff --git a/src/sfctl/config.py b/src/sfctl/config.py index 4a94ec65..c42d49df 100644 --- a/src/sfctl/config.py +++ b/src/sfctl/config.py @@ -31,7 +31,8 @@ def get_config_value(name, fallback=None): def get_config_bool(name, fallback=False): - """Checks if a config value is set to a valid bool value.""" + """Checks if a config value is set to a valid bool value. + Exception will be raised if the value is not convertible to True or False.""" cli_config = CLIConfig(SF_CLI_CONFIG_DIR, SF_CLI_ENV_VAR_PREFIX) return cli_config.getboolean('servicefabric', name, fallback) @@ -188,18 +189,21 @@ def set_telemetry_config(telemetry_on): :return: None """ if telemetry_on: - set_config_value('use_telemetry', 'true') + set_config_value('use_telemetry_v2', 'true') else: - set_config_value('use_telemetry', 'false') + set_config_value('use_telemetry_v2', 'false') def get_telemetry_config(): """ Gets whether or not telemetry is turned on Returns True if no value is set. - :return: bool. True if telemetry is on. False otherwise. + :return: bool. True if telemetry is on. False otherwise. Return None if no values are found. """ - return get_config_bool('use_telemetry', fallback=True) + if get_config_value('use_telemetry_v2') is None: + return None + + return get_config_bool('use_telemetry_v2') def get_cli_version_from_pkg(): diff --git a/src/sfctl/custom_cluster.py b/src/sfctl/custom_cluster.py index b125a7e7..37463470 100644 --- a/src/sfctl/custom_cluster.py +++ b/src/sfctl/custom_cluster.py @@ -260,7 +260,7 @@ def sfctl_cluster_version_matches(cluster_version, sfctl_version): :return: True if they are a match. False otherwise. """ - if sfctl_version in ['7.0.0', '7.0.1']: + if sfctl_version in ['7.0.0', '7.0.1', '7.0.2']: return cluster_version.startswith('6.4') diff --git a/src/sfctl/helps/settings.py b/src/sfctl/helps/settings.py index 93981f19..2e1f5717 100644 --- a/src/sfctl/helps/settings.py +++ b/src/sfctl/helps/settings.py @@ -26,7 +26,7 @@ short-summary: Turn off telemetry. - name: --on type: bool - short-summary: Turn on telemetry. This is the default value. + short-summary: Turn on telemetry. examples: diff --git a/src/sfctl/telemetry.py b/src/sfctl/telemetry.py index 902dcd1d..15198431 100644 --- a/src/sfctl/telemetry.py +++ b/src/sfctl/telemetry.py @@ -15,7 +15,7 @@ from uuid import uuid4 import portalocker from knack.log import get_logger -from sfctl.config import get_telemetry_config, get_cli_version_from_pkg +from sfctl.config import get_telemetry_config, set_telemetry_config, get_cli_version_from_pkg from sfctl.state import increment_telemetry_send_retry_count # knack CLIConfig has been re-purposed to handle state instead. @@ -49,8 +49,10 @@ def check_and_send_telemetry(args_list, invocation_ret_val, exception=None): :return: None """ + use_telemetry = get_telemetry_config() + # Only send telemetry if user has configured it to be on - if get_telemetry_config(): + if use_telemetry: try: @@ -85,6 +87,56 @@ def check_and_send_telemetry(args_list, invocation_ret_val, exception=None): logger.info( str.format('Not sending telemetry because python process due to error: {0}', ex)) + # Prompt the user if there is no value set in the config file for whether or not telemetry + # should be sent. If use telemetry is set to False, then do nothing. + elif use_telemetry is None: + + _prompt_for_telemetry(args_list, invocation_ret_val, exception) + + +def _prompt_for_telemetry(args_list, invocation_ret_val, exception): + """ + Ask users for a yes or no answer on collecting telemetry. + + If the user says yes, set the value in config, then re-run the check_and_send_telemetry check. + If the user says no, set the value in config and return. + + Inputs are the ones passed into check_and_send_telemetry. This just passes the information + along. + + :return: None + """ + + user_response = None + + while user_response not in ['y', 'yes', 'n', 'no']: + if user_response is None: # Means this is the first time asking for the prompt + prompt = str.format('Hello! In order for us to improve the user experience, ' + 'we would like to collect some data. Sfctl telemetry collects ' + 'command name without parameters ' + 'provided or their values, sfctl version, OS type, python version, ' + 'the success or failure of the command, the error message ' + 'returned.{0}' + 'To give permission, enter "y" or "yes". ' + 'To decline, enter "n" or "no": ', + os.linesep) + else: + prompt = ('Invalid response provided. Please enter "y" or "yes" to accept, ' + 'and "n" or "no" to decline: ') + + try: # raw_input is for python 2.x + user_response = raw_input(prompt) + except NameError: + user_response = input(prompt) + + if user_response.lower() in ['y', 'yes']: + set_telemetry_config(True) + # Re-run this function + check_and_send_telemetry(args_list, invocation_ret_val, exception) + + elif user_response.lower() in ['n', 'no']: + set_telemetry_config(False) + def batch_or_send_telemetry(command_as_str, command_return): """ diff --git a/src/sfctl/tests/request_generation_test.py b/src/sfctl/tests/request_generation_test.py index bbe298a0..699cd29d 100644 --- a/src/sfctl/tests/request_generation_test.py +++ b/src/sfctl/tests/request_generation_test.py @@ -11,6 +11,7 @@ from __future__ import print_function from os import (remove, path) +from random import choice import json import logging from shutil import rmtree @@ -215,6 +216,14 @@ def validate_command(self, command, method, url_path, query, body=None, # pylin # If this test reaches here, then this test is successful. remove(generated_file_path) # Clean up + @staticmethod + def _mock_raw_input(): + random_yes_or_no = choice([True, False]) + if random_yes_or_no: + return 'yes' + + return 'no' + def test_paths_generation(self): """ This test calls all commands that exist within sfctl. The commands are routed to a mock cluster which always returns @@ -222,11 +231,21 @@ def test_paths_generation(self): features to determine that the command is working as expected (generating the correct URL). """ - # Set test URL path to that of our mock server - set_mock_endpoint('http://localhost:' + str(self.port)) + try: # Python 3 + with patch('builtins.input', return_value=ServiceFabricRequestTests._mock_raw_input()): + # Set test URL path to that of our mock server + set_mock_endpoint('http://localhost:' + str(self.port)) + + # Call test + self.paths_generation_helper() + + except NameError: # Python 2 + with patch('__builtin__.raw_input', return_value=ServiceFabricRequestTests._mock_raw_input()): + # Set test URL path to that of our mock server + set_mock_endpoint('http://localhost:' + str(self.port)) - # Call test - self.paths_generation_helper() + # Call test + self.paths_generation_helper() def paths_generation_helper(self): # pylint: disable=too-many-statements, too-many-locals """ Lists all the commands to be tested and their expected values. diff --git a/src/sfctl/tests/version_test.py b/src/sfctl/tests/version_test.py index 735d2ba8..3864821b 100644 --- a/src/sfctl/tests/version_test.py +++ b/src/sfctl/tests/version_test.py @@ -16,7 +16,7 @@ def test_valid_current_version(self): note: this will require changing the sfctl_version on releases """ - sfctl_version = '7.0.1' + sfctl_version = '7.0.2' pipe = Popen('sfctl --version', shell=True, stdout=PIPE, stderr=PIPE) # returned_string and err are returned as bytes