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.

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

Delay os.getcwd() call to function body #243

Merged
merged 13 commits into from
May 27, 2024
Merged

Delay os.getcwd() call to function body #243

merged 13 commits into from
May 27, 2024

Conversation

dholth
Copy link
Contributor

@dholth dholth commented May 21, 2024 •
edited
Loading

Description

Fix #205 (if caller passes output directory)

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 May 21, 2024
@dholth dholth requested a review from kenodegard May 21, 2024 15:34
@dholth dholth enabled auto-merge (squash) May 21, 2024 15:44
@dholth dholth added the in-progress issue is actively being worked on label May 22, 2024
@jaimergp
Copy link
Contributor

The change looks ok to me but I'm not sure why this fixes the mentioned issue 🤔 How can the cwd not exist? Or is it because it's being run in a volatile directory which by the time of execution has been deleted?

@jaimergp jaimergp mentioned this pull request May 24, 2024
Closed
20 tasks
@dholth @jaimergp
Apply suggestions from code review
e33653d 
Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
@dholth
Copy link
Contributor Author

dholth commented May 24, 2024

The change looks ok to me but I'm not sure why this fixes the mentioned issue 🤔 How can the cwd not exist? Or is it because it's being run in a volatile directory which by the time of execution has been deleted?

Yes, it is a weird error. Could it be interacting with the cwd's lack of thread safety? This is the best we can do in conda-package-handling; if the caller passes out_folder then we won't trigger the exception.

mkdir foo
cd foo
python3
# in a separate shell, rmdir foo
>>> import os
>>> os.getcwd()
FileNotFoundError: [Errno 2] No such file or directory

@dholth dholth requested a review from jaimergp May 24, 2024 15:34
dholth
dholth commented May 24, 2024
View reviewed changes
pyproject.toml
@@ -5,3 +5,16 @@ line-length = 99
[tool.isort]
profile = "black"
line_length = 99

Copy link
Contributor Author

Choose a reason for hiding this comment

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

new hotness

setup.cfg
@@ -2,18 +2,3 @@
max-line-length = 100
ignore = E122,E123,E126,E127,E128,E731,E722
exclude = build,src/conda_package_handling/_version.py,tests,conda.recipe,.git,versioneer.py,benchmarks,.asv,rever

Copy link
Contributor Author

Choose a reason for hiding this comment

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

old n' busted

setup.cfg
--tb native
--strict-markers
--durations=20
env =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

avoid pytest-env requirement

tests/test_cli.py
@@ -55,6 +65,20 @@ def test_import_main():
)
def test_list(artifact, n_files, capsys):
"Integration test to ensure `cph list` works correctly."
cli.main(["list", os.path.join(data_dir, artifact)])
cli.main(["list", os.path.relpath(os.path.join(data_dir, artifact), os.getcwd())])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

code coverage for "is path relative? call abspath()" in list code

@dholth dholth merged commit 1f98026 into main May 27, 2024
15 checks passed
@dholth dholth deleted the 206-getcwd branch May 27, 2024 05:05
@dholth
Copy link
Contributor Author

dholth commented May 28, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaimergp jaimergp jaimergp approved these changes

@travishathaway travishathaway travishathaway approved these changes

@kenodegard kenodegard Awaiting requested review from kenodegard

Assignees
No one assigned
Labels
cla-signed [bot] added once the contributor has signed the CLA in-progress issue is actively being worked on
Projects
🔎 Review
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Build error related to os.getcwd() on linux. Only works with --no-build-id
4 participants
@dholth @jaimergp @travishathaway @conda-bot