Skip to content

Allow conda init --dev to output to either fd or tmp #11865

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Allow conda init --dev to output to either fd or tmp #11865

wants to merge 5 commits into from

Conversation

kenodegard
Copy link
Contributor

@kenodegard kenodegard commented Sep 20, 2022 •
edited
Loading

Description

Expose controlling where conda init --dev is output. There are three ways to output:

  • stdout (default)
  • a file descriptor (e.g., --fd 3), or
  • a (temp) file (e.g., --tmp $TMPDIR/conda-init-dev.$$.$RANDOM)

This way, the shell interface can choose how to receive the initialization script (as opposed to being told how it must receive it).

Depends on #11862

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Sep 20, 2022
@kenodegard kenodegard mentioned this pull request Sep 20, 2022
Closed
8 tasks
@kenodegard kenodegard force-pushed the init-dev-fd-tmp branch 3 times, most recently from 9898bdf to f1de5fb Compare September 28, 2022 23:26
@kenodegard kenodegard added the in-progress issue is actively being worked on label Sep 29, 2022
@kenodegard kenodegard marked this pull request as ready for review September 29, 2022 00:58
@kenodegard kenodegard requested a review from a team as a code owner September 29, 2022 00:58
jezdez
jezdez previously approved these changes Oct 4, 2022
View reviewed changes
Copy link
Member

@jezdez jezdez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for that refactor!

@kenodegard kenodegard force-pushed the init-dev-fd-tmp branch 2 times, most recently from 08e3349 to d041d9e Compare October 5, 2022 23:15
@kenodegard
Copy link
Contributor Author

pre-commit.ci autofix

@kenodegard kenodegard mentioned this pull request Oct 5, 2022
Merged
3 tasks
@kenodegard kenodegard force-pushed the init-dev-fd-tmp branch 2 times, most recently from 542db30 to e63d77f Compare October 6, 2022 00:30
kenodegard
kenodegard commented Oct 6, 2022
View reviewed changes
conda/cli/conda_argparse.py
Comment on lines +671 to +748
dev_piping_group = p.add_mutually_exclusive_group()
dev_piping_group.add_argument(
"--fd",
type=int,
help="When --dev is specified use --fd to pipe to a file descriptor besides stdout.",
default=None,
)
dev_piping_group.add_argument(
"--tmp",
type=Path,
help="When --dev is specified use --tmp to write to a temporary file instead of stdout.",
default=None,
)
Copy link
Contributor Author

@kenodegard kenodegard Oct 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining a mutex group where all of the args are SUPPRESS-ed breaks sphinx (but including help blurbs is better anyhow; less magic)...

@kenodegard kenodegard force-pushed the init-dev-fd-tmp branch 3 times, most recently from fe4c7b5 to fc2f891 Compare October 7, 2022 18:07
@kenodegard kenodegard mentioned this pull request Oct 7, 2022
Closed
@kenodegard kenodegard force-pushed the init-dev-fd-tmp branch 6 times, most recently from dc61689 to 16497fe Compare October 7, 2022 19:57
@kenodegard kenodegard force-pushed the init-dev-fd-tmp branch 6 times, most recently from f6fee9e to 5e31b78 Compare October 26, 2022 19:03
travishathaway
travishathaway requested changes Oct 27, 2022
View reviewed changes
Copy link
Contributor

@travishathaway travishathaway left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed there's a lot of uncovered lines. Is there a way for to test this even if we have to use something like subprocess.Popen? I would feel a little better about this pull request if we could bump up the test coverage.

I'll be happy to be persuaded otherwise though if this represents too big of a burden.

@kenodegard kenodegard mentioned this pull request Oct 27, 2022
Merged
3 tasks
@travishathaway travishathaway added the source::anaconda created by members of Anaconda, Inc. label Dec 5, 2022
@kenodegard kenodegard added backlog issue has been triaged but has not been earmarked for any upcoming release and removed in-progress issue is actively being worked on backlog issue has been triaged but has not been earmarked for any upcoming release labels Dec 12, 2022
@kenodegard
Copy link
Contributor Author

this was a valuable learning and research effort but is unlikely to proceed in the current form

@kenodegard kenodegard closed this Jun 15, 2023
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Jun 14, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@travishathaway travishathaway travishathaway requested changes

@jezdez jezdez jezdez left review comments

Assignees

@kenodegard kenodegard

Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity source::anaconda created by members of Anaconda, Inc.
Projects
🔎 Review
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kenodegard @jezdez @travishathaway @conda-bot