Skip to content
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

Inject environment variables for subprocess #38452

Open
morganics opened this issue Nov 11, 2024 · 6 comments
Open

Inject environment variables for subprocess #38452

morganics opened this issue Nov 11, 2024 · 6 comments
Assignees
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@morganics
Copy link

morganics commented Nov 11, 2024

Is your feature request related to a problem? Please describe.

Using os.environ can cause issues in azure_cli.py, e.g. cannot find the az command even when az is on the path.

Describe the solution you'd like
Inject os environment variables in to the identity module, so that we're not dependent on the global os.environ variables.

e.g. line 223 on azure_cli.py

        working_directory = get_safe_working_dir()

        kwargs: Dict[str, Any] = {
            "stderr": subprocess.PIPE,
            "stdin": subprocess.DEVNULL,
            "cwd": working_directory,
            "universal_newlines": True,
            "timeout": timeout,
            "env": dict(self._environ, AZURE_CORE_NO_COLOR="true"), # rather than using os.environ.
        }
        return subprocess.check_output(args, **kwargs)

Describe alternatives you've considered
Patching os.environ, which isn't great.

Additional context
Messy PATHs can stop the az command being found, despite shutil locating the appropriate path.

@github-actions github-actions bot added customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Nov 11, 2024
@xiangyan99 xiangyan99 removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Nov 11, 2024
@github-actions github-actions bot added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Nov 11, 2024
@xiangyan99
Copy link
Member

Thanks for the feedback, we’ll investigate asap.

@pvaneck
Copy link
Member

pvaneck commented Nov 19, 2024

Interesting that shutil.which can find az, but the subprocess is unable to resolve it even when passed in os.environ. From what I understand, shutil.which also retrieves the PATH environment variable from os.environ, which is the environment that should be passed to the subprocess. Something strange must be going on with the subprocess environment. I'm not sure if this stems from our usage of cmd \c and /bin/sh -c, however I can't seem to create a reproduction of the case where shutil can find az, but the subprocess cannot.

Are you encountering the issue on Windows or a Unix-based OS?

In any case, I am testing out just using the full path of the executable found from shutil.which, and just calling that directly. This approach should hopefully resolve the issue.

@xiangyan99 xiangyan99 added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Jan 15, 2025
Copy link

Hi @morganics. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@github-actions github-actions bot removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jan 15, 2025
@morganics
Copy link
Author

It's on Windows. Happy to test out when the changes land.

@github-actions github-actions bot added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels Jan 17, 2025
@pvaneck
Copy link
Member

pvaneck commented Feb 12, 2025

I believe the general issue here should be resolved in the latest release of azure-identity which includes the mentioned change: #38606

@pvaneck pvaneck added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Feb 12, 2025
@github-actions github-actions bot removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Feb 12, 2025
Copy link

Hi @morganics. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

3 participants