You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Description
Added a fallback order equivalent to the one found in the requests package. Also removed the unneeded setdefaults. See #21807 for history, and the recent comment done by @jiasli
src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource_bicep.py:65: src/azure-cli/azure/cli/command_modules/resource/bicep.py:116: in ensure_bicep_installation installed_version = get_bicep_installed_version(installation_path) src/azure-cli/azure/cli/command_modules/resource/bicep.py:264: in get_bicep_installed_version installed_version_output = run_command(bicep_executable_path, ["--version"]) src/azure-cli/azure/cli/command_modules/resource/bicep.py:302: in run_command process = subprocess.run( /opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/subprocess.py:548: in run with Popen(*popenargs, **kwargs) as process: /opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/subprocess.py:1026: in init self.execute_child(args, executable, preexec_fn, close_fds, _
if isinstance(args, (str, bytes)): args = [args] elif isinstance(args, os.PathLike): if shell: raise TypeError('path-like args is not allowed when ' 'shell is true') args = [args] else: args = list(args)
if shell: # On Android the default shell is at '/system/bin/sh'. unix_shell = ('/system/bin/sh' if hasattr(sys, 'getandroidapilevel') else '/bin/sh') args = [unix_shell, "-c"] + args if executable: args[0] = executable
if (_USE_POSIX_SPAWN and os.path.dirname(executable) and preexec_fn is None and not close_fds and not pass_fds and cwd is None and (p2cread == -1 or p2cread > 2) and (c2pwrite == -1 or c2pwrite > 2) and (errwrite == -1 or errwrite > 2) and not start_new_session and process_group == -1 and gid is None and gids is None and uid is None and umask < 0): self._posix_spawn(args, executable, env, restore_signals, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite) return
orig_executable = executable
# For transferring possible exec failure from child to parent. # Data format: "exception name:hex errno:description" # Pickle is not used; it is complex and involves memory allocation. errpipe_read, errpipe_write = os.pipe() # errpipe_write must not be in the standard io 0, 1, or 2 fd range. low_fds_to_close = [] while errpipe_write < 3: low_fds_to_close.append(errpipe_write) errpipe_write = os.dup(errpipe_write) for low_fd in low_fds_to_close: os.close(low_fd) try: try: # We must avoid complex work that could involve # malloc or free in the child process to avoid # potential deadlocks, thus we do all this here. # and pass it to fork_exec()
if env is not None: env_list = [] for k, v in env.items(): k = os.fsencode(k) if b'=' in k: raise ValueError("illegal environment variable name") env_list.append(k + b'=' + os.fsencode(v)) else: env_list = None # Use execv instead of execve. executable = os.fsencode(executable) if os.path.dirname(executable): executable_list = (executable,) else: # This matches the behavior of os._execvpe(). executable_list = tuple( os.path.join(os.fsencode(dir), executable) for dir in os.get_exec_path(env)) fds_to_keep = set(pass_fds) fds_to_keep.add(errpipe_write) self.pid = _fork_exec( args, executable_list, close_fds, tuple(sorted(map(int, fds_to_keep))), cwd, env_list, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite, errpipe_read, errpipe_write, restore_signals, start_new_session, process_group, gid, gids, uid, umask, preexec_fn, _USE_VFORK) self._child_created = True finally: # be sure the FD is closed no matter what os.close(errpipe_write)
# Wait for exec to fail or succeed; possibly raising an # exception (limited in size) errpipe_data = bytearray() while True: part = os.read(errpipe_read, 50000) errpipe_data += part if not part or len(errpipe_data) > 50000: break finally: # be sure the FD is closed no matter what os.close(errpipe_read)
if errpipe_data: try: pid, sts = os.waitpid(self.pid, 0) if pid == self.pid: self._handle_exitstatus(sts) else: self.returncode = sys.maxsize except ChildProcessError: pass
try: exception_name, hex_errno, err_msg = ( errpipe_data.split(b':', 2)) # The encoding here should match the encoding # written in by the subprocess implementations # like _posixsubprocess err_msg = err_msg.decode() except ValueError: exception_name = b'SubprocessError' hex_errno = b'0' err_msg = 'Bad exception data from child: {!r}'.format( bytes(errpipe_data)) child_exception_type = getattr( builtins, exception_name.decode('ascii'), SubprocessError) if issubclass(child_exception_type, OSError) and hex_errno: errno_num = int(hex_errno, 16) if err_msg == "noexec:chdir": err_msg = "" # The error must be from chdir(cwd). err_filename = cwd elif err_msg == "noexec": err_msg = "" err_filename = None else: err_filename = orig_executable if errno_num != 0: err_msg = os.strerror(errno_num) if err_filename is not None: > raise child_exception_type(errno_num, err_msg, err_filename) E FileNotFoundError: [Errno 2] No such file or directory: '/home/cloudtest/.azure/bin/bicep'
src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource_bicep.py:65: src/azure-cli/azure/cli/command_modules/resource/bicep.py:116: in ensure_bicep_installation installed_version = get_bicep_installed_version(installation_path) src/azure-cli/azure/cli/command_modules/resource/bicep.py:264: in get_bicep_installed_version installed_version_output = run_command(bicep_executable_path, ["--version"]) src/azure-cli/azure/cli/command_modules/resource/bicep.py:302: in run_command process = subprocess.run( /opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/subprocess.py:505: in run with Popen(*popenargs, **kwargs) as process: /opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/subprocess.py:951: in init self.execute_child(args, executable, preexec_fn, close_fds, _
if isinstance(args, (str, bytes)): args = [args] elif isinstance(args, os.PathLike): if shell: raise TypeError('path-like args is not allowed when ' 'shell is true') args = [args] else: args = list(args)
if shell: # On Android the default shell is at '/system/bin/sh'. unix_shell = ('/system/bin/sh' if hasattr(sys, 'getandroidapilevel') else '/bin/sh') args = [unix_shell, "-c"] + args if executable: args[0] = executable
if (_USE_POSIX_SPAWN and os.path.dirname(executable) and preexec_fn is None and not close_fds and not pass_fds and cwd is None and (p2cread == -1 or p2cread > 2) and (c2pwrite == -1 or c2pwrite > 2) and (errwrite == -1 or errwrite > 2) and not start_new_session and gid is None and gids is None and uid is None and umask < 0): self._posix_spawn(args, executable, env, restore_signals, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite) return
orig_executable = executable
# For transferring possible exec failure from child to parent. # Data format: "exception name:hex errno:description" # Pickle is not used; it is complex and involves memory allocation. errpipe_read, errpipe_write = os.pipe() # errpipe_write must not be in the standard io 0, 1, or 2 fd range. low_fds_to_close = [] while errpipe_write < 3: low_fds_to_close.append(errpipe_write) errpipe_write = os.dup(errpipe_write) for low_fd in low_fds_to_close: os.close(low_fd) try: try: # We must avoid complex work that could involve # malloc or free in the child process to avoid # potential deadlocks, thus we do all this here. # and pass it to fork_exec()
if env is not None: env_list = [] for k, v in env.items(): k = os.fsencode(k) if b'=' in k: raise ValueError("illegal environment variable name") env_list.append(k + b'=' + os.fsencode(v)) else: env_list = None # Use execv instead of execve. executable = os.fsencode(executable) if os.path.dirname(executable): executable_list = (executable,) else: # This matches the behavior of os._execvpe(). executable_list = tuple( os.path.join(os.fsencode(dir), executable) for dir in os.get_exec_path(env)) fds_to_keep = set(pass_fds) fds_to_keep.add(errpipe_write) self.pid = _posixsubprocess.fork_exec( args, executable_list, close_fds, tuple(sorted(map(int, fds_to_keep))), cwd, env_list, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite, errpipe_read, errpipe_write, restore_signals, start_new_session, gid, gids, uid, umask, preexec_fn) self._child_created = True finally: # be sure the FD is closed no matter what os.close(errpipe_write)
# Wait for exec to fail or succeed; possibly raising an # exception (limited in size) errpipe_data = bytearray() while True: part = os.read(errpipe_read, 50000) errpipe_data += part if not part or len(errpipe_data) > 50000: break finally: # be sure the FD is closed no matter what os.close(errpipe_read)
if errpipe_data: try: pid, sts = os.waitpid(self.pid, 0) if pid == self.pid: self._handle_exitstatus(sts) else: self.returncode = sys.maxsize except ChildProcessError: pass
try: exception_name, hex_errno, err_msg = ( errpipe_data.split(b':', 2)) # The encoding here should match the encoding # written in by the subprocess implementations # like _posixsubprocess err_msg = err_msg.decode() except ValueError: exception_name = b'SubprocessError' hex_errno = b'0' err_msg = 'Bad exception data from child: {!r}'.format( bytes(errpipe_data)) child_exception_type = getattr( builtins, exception_name.decode('ascii'), SubprocessError) if issubclass(child_exception_type, OSError) and hex_errno: errno_num = int(hex_errno, 16) child_exec_never_called = (err_msg == "noexec") if child_exec_never_called: err_msg = "" # The error must be from chdir(cwd). err_filename = cwd else: err_filename = orig_executable if errno_num != 0: err_msg = os.strerror(errno_num) > raise child_exception_type(errno_num, err_msg, err_filename) E FileNotFoundError: [Errno 2] No such file or directory: '/home/cloudtest/.azure/bin/bicep'
@zhoxing-ms Could you take a look at the PR? Thanks.
@wwmoraes It seems there are code style errors to fix. The CI logs expired, but you can see the errors locally by running azdev style resource and azdev linter resource (suppose you have azdev configured according to the contribution guide).
The reason will be displayed to describe this comment to others. Learn more.
Could we consider using requests.get instead of urlopen, which may make the code more concise and consistent with the approach of get_bicep_available_release_tags() and get_bicep_latest_release_tag()
Running flake8 on modules...
ERROR: /mnt/vss/_work/1/s/src/azure-cli/azure/cli/command_modules/resource/_bicep.py:136:11: E121 continuation line under-indented for hanging indent
/mnt/vss/_work/1/s/src/azure-cli/azure/cli/command_modules/resource/_bicep.py:137:11: W503 line break before binary operator
/mnt/vss/_work/1/s/src/azure-cli/azure/cli/command_modules/resource/_bicep.py:138:11: W503 line break before binary operator
/mnt/vss/_work/1/s/src/azure-cli/azure/cli/command_modules/resource/_bicep.py:136:11: E121 continuation line under-indented for hanging indent
/mnt/vss/_work/1/s/src/azure-cli/azure/cli/command_modules/resource/_bicep.py:137:11: W503 line break before binary operator
/mnt/vss/_work/1/s/src/azure-cli/azure/cli/command_modules/resource/_bicep.py:138:11: W503 line break before binary operator
2 E121 continuation line under-indented for hanging indent
4 W503 line break before binary operator
Could you please resolve these flake8 related issues?
The individual test case reported as an error works fine when ran directly with azdev test test_use_bicep_cli_from_path_false_after_install and with a pre-existing binary in the path. It fails when ran without an installed bicep version or if the entire suite is ran as azdev test TestBicep, with or without a binary on the path. Running the tests with --series does not solve it either. This relates to how the test functions do not run within an isolated temporary directory. This means all tests share the environment, which leads to unpredictable behaviour like the one seen here. It doubles down as the test depends on a pre-existing environment state, which makes the test non-atomic and non-reproducible.
It is beyond the objective of this PR to refactor this entire suite of unit tests. @zhoxing-ms would you be so kind as to review it once again?
I guess we can solve this CI problem through mock _get_bicep_installation_path and _get_bicep_installed_version. But sorry, due to recent busy work, I don't have enough time to validate this idea.
Type Test Case Error Message Line
Failed test_use_bicep_cli_from_path_false_after_install E FileNotFoundError: [Errno 2] No such file or directory: '/home/cloudtest/.azure/bin/bicep' azure/cli/command_modules/resource/tests/latest/test_resource_bicep.py:36
I guess we can solve this CI problem through mock _get_bicep_installation_path and _get_bicep_installed_version
@shenglol Do you have any thoughts or suggestions regarding this testing issue?
It seems the os.path.isfile stub is not working as expected. It is set to return False, so it should force the first if block in ensure_bicep_installation to be skipped, making no calls to _get_bicep_installed_version. Yet we see it being called and erroring during tests. 🤷♂️
I think the errors are due to test cases sharing the same DummyCli instance, resulting in a race condition. I've committed a fix in #26797. @wwmoraes You may integrate the changes once that PR is merged to get unblocked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
act-identity-squadARMaz resource/group/lock/tag/deployment/policy/managementapp/account management-groupAuto-AssignAuto assign by botcustomer-reportedIssues that are reported by GitHub users external to the Azure organization.
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related command
az bicep installDescription
Added a fallback order equivalent to the one found in the requests package. Also removed the unneeded setdefaults. See #21807 for history, and the recent comment done by @jiasli
closes #26007
Testing Guide
A clean environment should use the certifi CA bundle path:
If either
REQUESTS_CA_BUNDLEorCURL_CA_BUNDLEis set, then it should be picked up:If both
REQUESTS_CA_BUNDLEandCURL_CA_BUNDLEare set, then it should useREQUESTS_CA_BUNDLE(this is by definition of the requests package).History Notes
[ARM]
az bicep install: Use CA bundle environment variables if setThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.