From 32bd3efa2d6b7d4cc22c6c8e55cc5b3ba14c2b5e Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 5 Oct 2022 16:21:55 -0700 Subject: [PATCH 01/12] add updated logic to resolve assets.json and pass along to record|playback/start --- .gitignore | 1 + .../devtools_testutils/proxy_startup.py | 8 +++++ .../devtools_testutils/proxy_testcase.py | 31 +++++++++++++++++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index e6e614745b6d..cfb9c024301b 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,7 @@ build/ # Test results TestResults/ ENV_DIR/ +.assets/ # Perf test profiling cProfile-*.pstats diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py index 4bda8fe0ac25..3ecc44a08e39 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py @@ -31,6 +31,7 @@ PROXY_CHECK_URL = PROXY_URL.rstrip("/") + "/Info/Available" TOOL_ENV_VAR = "PROXY_PID" +discovered_roots = [] def get_image_tag(repo_root: str) -> str: """Gets the test proxy Docker image tag from the target_version.txt file in /eng/common/testproxy""" @@ -59,6 +60,8 @@ def ascend_to_root(start_dir_or_file: str) -> str: # we need the git check to prevent ascending out of the repo if os.path.exists(possible_root): + if current_dir not in discovered_roots: + discovered_roots.append(current_dir) return current_dir else: current_dir = os.path.dirname(current_dir) @@ -137,13 +140,18 @@ def start_test_proxy(request) -> None: else: envname = os.getenv("TOX_ENV_NAME", "default") root = os.getenv("BUILD_SOURCESDIRECTORY", repo_root) + + + log = open(os.path.join(root, "_proxy_log_{}.log".format(envname)), "a") + env = {} _LOGGER.info("{} is calculated repo root".format(root)) proc = subprocess.Popen( shlex.split('test-proxy start --storage-location="{}" -- --urls "{}"'.format(root, PROXY_URL)), stdout=log, stderr=log, + env = {} ) os.environ[TOOL_ENV_VAR] = str(proc.pid) else: diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py b/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py index 1d7954fae911..715b0352b37b 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py @@ -6,6 +6,7 @@ import logging import requests import six +import os from typing import TYPE_CHECKING import urllib.parse as url_parse @@ -19,6 +20,7 @@ from azure_devtools.scenario_tests.utilities import trim_kwargs_from_test_function from .config import PROXY_URL from .helpers import get_test_id, is_live, is_live_and_not_recording, set_recording_id +from .proxy_startup import discovered_roots if TYPE_CHECKING: from typing import Callable, Dict, Tuple @@ -34,6 +36,26 @@ PLAYBACK_START_URL = "{}/playback/start".format(PROXY_URL) PLAYBACK_STOP_URL = "{}/playback/stop".format(PROXY_URL) +def get_recording_assets(test_id: str) -> str: + """ + Used to retrieve the assets.json given a PYTEST_CURRENT_TEST test id. + """ + for root in discovered_roots: + current_dir = os.path.dirname(test_id) + while current_dir is not None and not (os.path.dirname(current_dir) == current_dir): + possible_assets = os.path.join(current_dir, "assets.json") + possible_root = os.path.join(current_dir, ".git") + + # we need to check for assets.json first! + if os.path.exists(os.path.join(root, possible_assets)): + return os.path.abspath(os.path.join(root, possible_assets)) + # we need the git check to prevent ascending out of the repo + elif os.path.exists(os.path.join(root, possible_root)): + return None + else: + current_dir = os.path.dirname(current_dir) + + return None def start_record_or_playback(test_id: str) -> "Tuple[str, Dict[str, str]]": """Sends a request to begin recording or playing back the provided test. @@ -42,11 +64,16 @@ def start_record_or_playback(test_id: str) -> "Tuple[str, Dict[str, str]]": test variables to values. If no variable dictionary was stored when the test was recorded, b is an empty dictionary. """ variables = {} # this stores a dictionary of test variable values that could have been stored with a recording + + json_payload = {"x-recording-file": test_id} + assets_json = get_recording_assets(test_id) + if assets_json: + json_payload["x-recording-assets-file"] = assets_json if is_live(): result = requests.post( RECORDING_START_URL, - json={"x-recording-file": test_id}, + json=json_payload, ) if result.status_code != 200: message = six.ensure_str(result._content) @@ -56,7 +83,7 @@ def start_record_or_playback(test_id: str) -> "Tuple[str, Dict[str, str]]": else: result = requests.post( PLAYBACK_START_URL, - json={"x-recording-file": test_id}, + json=json_payload, ) if result.status_code != 200: message = six.ensure_str(result._content) From b3d4fcff145799f272952c8a513de82bcd94d8ca Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 5 Oct 2022 16:25:34 -0700 Subject: [PATCH 02/12] ensure that parallel tox environments don't accidentally stomp on each other --- tools/azure-sdk-tools/devtools_testutils/proxy_startup.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py index 3ecc44a08e39..5e2299f3691e 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py @@ -140,18 +140,16 @@ def start_test_proxy(request) -> None: else: envname = os.getenv("TOX_ENV_NAME", "default") root = os.getenv("BUILD_SOURCESDIRECTORY", repo_root) - - - log = open(os.path.join(root, "_proxy_log_{}.log".format(envname)), "a") - env = {} _LOGGER.info("{} is calculated repo root".format(root)) proc = subprocess.Popen( shlex.split('test-proxy start --storage-location="{}" -- --urls "{}"'.format(root, PROXY_URL)), stdout=log, stderr=log, - env = {} + env = { + "PROXY_ASSETS_FOLDER": os.path.join(root, envname) + } ) os.environ[TOOL_ENV_VAR] = str(proc.pid) else: From aeed849727889a3330e7488d8fe4846bbc8e7d9b Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 5 Oct 2022 17:11:23 -0700 Subject: [PATCH 03/12] update where the local proxy assets are stored during CI runs --- tools/azure-sdk-tools/devtools_testutils/proxy_startup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py index 5e2299f3691e..c36c64c2ce4f 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py @@ -143,12 +143,14 @@ def start_test_proxy(request) -> None: log = open(os.path.join(root, "_proxy_log_{}.log".format(envname)), "a") _LOGGER.info("{} is calculated repo root".format(root)) + + os.environ["PROXY_ASSETS_FOLDER"] = os.path.join(root, envname) + proc = subprocess.Popen( shlex.split('test-proxy start --storage-location="{}" -- --urls "{}"'.format(root, PROXY_URL)), stdout=log, stderr=log, env = { - "PROXY_ASSETS_FOLDER": os.path.join(root, envname) } ) os.environ[TOOL_ENV_VAR] = str(proc.pid) From 877e8a3c8607e4b4e4fae6ba4b9e6483dd8ce43e Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 5 Oct 2022 18:17:21 -0700 Subject: [PATCH 04/12] add an assets.json generated by the transition script --- sdk/tables/assets.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 sdk/tables/assets.json diff --git a/sdk/tables/assets.json b/sdk/tables/assets.json new file mode 100644 index 000000000000..8e0eed27dc69 --- /dev/null +++ b/sdk/tables/assets.json @@ -0,0 +1,6 @@ +{ + "AssetsRepo": "Azure/azure-sdk-assets", + "AssetsRepoPrefixPath": "python", + "TagPrefix": "python/tables", + "Tag": "python/tablesf365d217-f0e2-4d01-be57-4d61cc54e966" +} From 7b9b88c384d35967312e72f00728e62dab0b669b Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 12 Oct 2022 14:07:51 -0700 Subject: [PATCH 05/12] set environment variable and get rolling. clean up after the log directory. --- scripts/devops_tasks/tox_harness.py | 7 +++++-- tools/azure-sdk-tools/devtools_testutils/proxy_startup.py | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/scripts/devops_tasks/tox_harness.py b/scripts/devops_tasks/tox_harness.py index de37e6a19b9c..73053d658eeb 100644 --- a/scripts/devops_tasks/tox_harness.py +++ b/scripts/devops_tasks/tox_harness.py @@ -165,7 +165,6 @@ def individual_workload(tox_command_tuple, workload_results): if in_ci(): shutil.rmtree(tox_dir) - def execute_tox_parallel(tox_command_tuples): pool = ThreadPool(pool_size) workload_results = {} @@ -256,6 +255,7 @@ def build_whl_for_req(req, package_path): else: return req + def replace_dev_reqs(file, pkg_root): adjusted_req_lines = [] @@ -355,12 +355,12 @@ def collect_log_files(working_dir): for f in glob.glob(os.path.join(root_dir, "_tox_logs", "*")): logging.info("Log file: {}".format(f)) - def execute_tox_serial(tox_command_tuples): return_code = 0 for index, cmd_tuple in enumerate(tox_command_tuples): tox_dir = os.path.abspath(os.path.join(cmd_tuple[1], "./.tox/")) + clone_dir = os.path.abspath(os.path.join(cmd_tuple[1], "..", "..", "..", "l")) logging.info("tox_dir: {}".format(tox_dir)) logging.info( @@ -378,6 +378,9 @@ def execute_tox_serial(tox_command_tuples): collect_log_files(cmd_tuple[1]) shutil.rmtree(tox_dir) + if os.path.exists(clone_dir): + shutil.rmtree(clone_dir) + return return_code diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py index c36c64c2ce4f..f3ac83d1277a 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py @@ -144,14 +144,14 @@ def start_test_proxy(request) -> None: _LOGGER.info("{} is calculated repo root".format(root)) - os.environ["PROXY_ASSETS_FOLDER"] = os.path.join(root, envname) + os.environ["PROXY_ASSETS_FOLDER"] = os.path.join(root, "l", envname) + if not os.path.exists(os.environ["PROXY_ASSETS_FOLDER"]): + os.mkdir(os.environ["PROXY_ASSETS_FOLDER"]) proc = subprocess.Popen( shlex.split('test-proxy start --storage-location="{}" -- --urls "{}"'.format(root, PROXY_URL)), stdout=log, - stderr=log, - env = { - } + stderr=log ) os.environ[TOOL_ENV_VAR] = str(proc.pid) else: From a3518898ab7d6f53abd275266e3755ecac6840aa Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 12 Oct 2022 15:28:39 -0700 Subject: [PATCH 06/12] ensure that we properly set that proxy server setting if the directory does not exist --- tools/azure-sdk-tools/devtools_testutils/proxy_startup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py index f3ac83d1277a..9a81ed6e8c27 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py @@ -146,7 +146,7 @@ def start_test_proxy(request) -> None: os.environ["PROXY_ASSETS_FOLDER"] = os.path.join(root, "l", envname) if not os.path.exists(os.environ["PROXY_ASSETS_FOLDER"]): - os.mkdir(os.environ["PROXY_ASSETS_FOLDER"]) + os.mkdirs(os.environ["PROXY_ASSETS_FOLDER"]) proc = subprocess.Popen( shlex.split('test-proxy start --storage-location="{}" -- --urls "{}"'.format(root, PROXY_URL)), From 524878519754f948e6156d8ad5c110df75189b20 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 12 Oct 2022 15:31:22 -0700 Subject: [PATCH 07/12] target renewed tag that should have all the files --- sdk/tables/assets.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/tables/assets.json b/sdk/tables/assets.json index 8e0eed27dc69..724b94ce610c 100644 --- a/sdk/tables/assets.json +++ b/sdk/tables/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "python", "TagPrefix": "python/tables", - "Tag": "python/tablesf365d217-f0e2-4d01-be57-4d61cc54e966" + "Tag": "python/tables_26ba55dfd5" } From 544d292ecdab60707675f5f092ff18ae7e63b702 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 12 Oct 2022 15:54:52 -0700 Subject: [PATCH 08/12] mkdirs -> makedirs --- tools/azure-sdk-tools/devtools_testutils/proxy_startup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py index 9a81ed6e8c27..d12448ef674f 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py @@ -146,7 +146,7 @@ def start_test_proxy(request) -> None: os.environ["PROXY_ASSETS_FOLDER"] = os.path.join(root, "l", envname) if not os.path.exists(os.environ["PROXY_ASSETS_FOLDER"]): - os.mkdirs(os.environ["PROXY_ASSETS_FOLDER"]) + os.makedirs(os.environ["PROXY_ASSETS_FOLDER"]) proc = subprocess.Popen( shlex.split('test-proxy start --storage-location="{}" -- --urls "{}"'.format(root, PROXY_URL)), From 9e16bbe07b17a9c01e4fcb048efd2fb91f7ba959 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 12 Oct 2022 17:41:26 -0700 Subject: [PATCH 09/12] resolve issue with cleanup in windows machines --- scripts/devops_tasks/tox_harness.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/scripts/devops_tasks/tox_harness.py b/scripts/devops_tasks/tox_harness.py index 73053d658eeb..e495e4157699 100644 --- a/scripts/devops_tasks/tox_harness.py +++ b/scripts/devops_tasks/tox_harness.py @@ -379,7 +379,14 @@ def execute_tox_serial(tox_command_tuples): shutil.rmtree(tox_dir) if os.path.exists(clone_dir): - shutil.rmtree(clone_dir) + try: + shutil.rmtree(clone_dir) + except Exception as e: + # git has a permissions problem. one of the files it drops + # cannot be removed as no one has the permission to do so. + # lets log just in case, but this should really only affect windows machines. + logging.info(e) + pass return return_code From 41562efd7a7864442b35c8c826197c20e9a05262 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Fri, 14 Oct 2022 11:52:51 -0700 Subject: [PATCH 10/12] permanently address the path issues that mccoy was just seeing. --- tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py b/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py index 715b0352b37b..a3a94cdaee38 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py @@ -48,7 +48,8 @@ def get_recording_assets(test_id: str) -> str: # we need to check for assets.json first! if os.path.exists(os.path.join(root, possible_assets)): - return os.path.abspath(os.path.join(root, possible_assets)) + complete_path = os.path.abspath(os.path.join(root, possible_assets)) + return os.path.relpath(complete_path, root) # we need the git check to prevent ascending out of the repo elif os.path.exists(os.path.join(root, possible_root)): return None From fc77f5267616713f86f513c84c32b7c303d60688 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?McCoy=20Pati=C3=B1o?= <39780829+mccoyp@users.noreply.github.com> Date: Fri, 14 Oct 2022 12:39:26 -0700 Subject: [PATCH 11/12] Reformat path for windows compat --- tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py b/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py index a3a94cdaee38..0f68130a5455 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py @@ -49,7 +49,7 @@ def get_recording_assets(test_id: str) -> str: # we need to check for assets.json first! if os.path.exists(os.path.join(root, possible_assets)): complete_path = os.path.abspath(os.path.join(root, possible_assets)) - return os.path.relpath(complete_path, root) + return os.path.relpath(complete_path, root).replace("\\", "/") # we need the git check to prevent ascending out of the repo elif os.path.exists(os.path.join(root, possible_root)): return None From f0c830c5a3009d2e37fe119289791505be7fd232 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Fri, 14 Oct 2022 12:42:07 -0700 Subject: [PATCH 12/12] removing the tables assets.json. leaving that up to Yalin to actually migrate --- sdk/tables/assets.json | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 sdk/tables/assets.json diff --git a/sdk/tables/assets.json b/sdk/tables/assets.json deleted file mode 100644 index 724b94ce610c..000000000000 --- a/sdk/tables/assets.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "AssetsRepo": "Azure/azure-sdk-assets", - "AssetsRepoPrefixPath": "python", - "TagPrefix": "python/tables", - "Tag": "python/tables_26ba55dfd5" -}