From eb62f19ffc7d65b912fa6030fd4a5ac356b4355a Mon Sep 17 00:00:00 2001 From: Christina-Kang Date: Tue, 18 Dec 2018 16:21:50 -0800 Subject: [PATCH 1/4] add prompting for telemetry and prepare for release 7.0.2 --- src/README.rst | 4 +++ src/setup.py | 2 +- src/sfctl/config.py | 14 +++++---- src/sfctl/custom_cluster.py | 2 +- src/sfctl/helps/settings.py | 2 +- src/sfctl/telemetry.py | 53 +++++++++++++++++++++++++++++++-- src/sfctl/tests/version_test.py | 2 +- 7 files changed, 68 insertions(+), 11 deletions(-) diff --git a/src/README.rst b/src/README.rst index c6b9d495..b8c235c9 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 (#) + 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..3bdcb63d 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. Turn 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..9e710a09 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 == True: try: @@ -85,6 +87,53 @@ 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 = '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. ' \ + + os.linesep + \ + 'To give permission, enter "y" or "yes". To decline, enter "n" or "no": ' + 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/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 From 30c3de6955a3147e459ac229ad4282b65c02697c Mon Sep 17 00:00:00 2001 From: Christina-Kang Date: Tue, 18 Dec 2018 17:04:25 -0800 Subject: [PATCH 2/4] give raw input to tests --- src/sfctl/config.py | 2 +- src/sfctl/tests/request_generation_test.py | 27 ++++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/sfctl/config.py b/src/sfctl/config.py index 3bdcb63d..de2d77c9 100644 --- a/src/sfctl/config.py +++ b/src/sfctl/config.py @@ -198,7 +198,7 @@ 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. Turn None if no values are found. + :return: bool. True if telemetry is on. False otherwise. Return None if no values are found. """ if get_config_value('use_telemetry_v2') is None: return None diff --git a/src/sfctl/tests/request_generation_test.py b/src/sfctl/tests/request_generation_test.py index bbe298a0..8fcd6a58 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.raw_input', return_value=ServiceFabricRequestTests._mock_raw_input()) as input_mock: + # 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: # Python 2 + with patch('__builtin__.input', return_value=ServiceFabricRequestTests._mock_raw_input()) as input_mock: + # 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. From 814036a6eda41737d48b4b6e5d001d25fe908efc Mon Sep 17 00:00:00 2001 From: Christina-Kang Date: Wed, 19 Dec 2018 11:52:16 -0800 Subject: [PATCH 3/4] lint --- src/sfctl/telemetry.py | 21 ++++++++++++--------- src/sfctl/tests/request_generation_test.py | 6 +++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/sfctl/telemetry.py b/src/sfctl/telemetry.py index 9e710a09..15198431 100644 --- a/src/sfctl/telemetry.py +++ b/src/sfctl/telemetry.py @@ -52,7 +52,7 @@ def check_and_send_telemetry(args_list, invocation_ret_val, exception=None): use_telemetry = get_telemetry_config() # Only send telemetry if user has configured it to be on - if use_telemetry == True: + if use_telemetry: try: @@ -111,15 +111,18 @@ def _prompt_for_telemetry(args_list, invocation_ret_val, exception): 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 = '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. ' \ - + os.linesep + \ - 'To give permission, enter "y" or "yes". To decline, enter "n" or "no": ' + 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..' + 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) diff --git a/src/sfctl/tests/request_generation_test.py b/src/sfctl/tests/request_generation_test.py index 8fcd6a58..699cd29d 100644 --- a/src/sfctl/tests/request_generation_test.py +++ b/src/sfctl/tests/request_generation_test.py @@ -232,15 +232,15 @@ def test_paths_generation(self): (generating the correct URL). """ try: # Python 3 - with patch('builtins.raw_input', return_value=ServiceFabricRequestTests._mock_raw_input()) as input_mock: + 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: # Python 2 - with patch('__builtin__.input', return_value=ServiceFabricRequestTests._mock_raw_input()) as input_mock: + 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)) From 8c08df182d53e16a112cc16e11eec170c8bff5d7 Mon Sep 17 00:00:00 2001 From: Christina-Kang Date: Wed, 19 Dec 2018 13:29:41 -0800 Subject: [PATCH 4/4] minor changes --- src/README.rst | 2 +- src/sfctl/config.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/README.rst b/src/README.rst index b8c235c9..99414121 100644 --- a/src/README.rst +++ b/src/README.rst @@ -18,7 +18,7 @@ Change Log 7.0.2 ---------- -- Prompt on first use for telemetry. Update code for 7.0.2 release (#) +- Prompt on first use for telemetry. Update code for 7.0.2 release (#170) 7.0.1 ---------- diff --git a/src/sfctl/config.py b/src/sfctl/config.py index de2d77c9..c42d49df 100644 --- a/src/sfctl/config.py +++ b/src/sfctl/config.py @@ -32,7 +32,7 @@ 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. - Exception will be raised if the value is not convertible to true or false.""" + 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)