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

fix(launch): allow spaces in requirements file, remove duplicate wandb bootstrap file #4647

Merged
merged 3 commits into from
Dec 20, 2022

Conversation

TimH98
Copy link
Contributor

@TimH98 TimH98 commented Dec 15, 2022

Fixes WB-11794

Description

_wandb_bootstrap.py was failing to handle the case where a requirement is specified as xyz @ ... due to spaces being present. This change removes the spaces, fixing the behavior.

Also removed sdk/launch/templates/_wandb_bootstrap.py, since it was a duplicate of sdk/launch/builder/templates/_wandb_bootstrap.py

Testing

  1. cd to wandb/sdk/launch/builder/templates/
  2. Create a requirements.frozen.txt file with the contents:
python-package-example
example-pypi-package @ https://files.pythonhosted.org/packages/93/8d/e1e98360dc899e533cb3dd857494f2571b129bdffcee76365009b2bb507c/example_pypi_package-0.1.0.tar.gz
python-example-package
  1. Run python _wandb_bootstrap.py
  2. Run pip list | grep 'example', confirm that the three example packages were installed
  3. Cleanup: Run pip uninstall -r requirements.frozen.txt -y

Also ran tox -e pylaunch38

@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Merging #4647 (d092f86) into main (709a60d) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head d092f86 differs from pull request most recent head 065cc7c. Consider uploading reports for the commit 065cc7c to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4647      +/-   ##
==========================================
- Coverage   83.22%   83.22%   -0.01%     
==========================================
  Files         279      279              
  Lines       34917    34917              
==========================================
- Hits        29060    29058       -2     
- Misses       5857     5859       +2     
Flag Coverage Δ
functest 56.08% <ø> (+0.04%) ⬆️
unittest 73.77% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
wandb/util.py 87.44% <0.00%> (-0.41%) ⬇️
wandb/sdk/wandb_run.py 90.66% <0.00%> (-0.24%) ⬇️
wandb/sdk/lib/git.py 77.71% <0.00%> (ø)
wandb/filesync/step_prepare.py 94.93% <0.00%> (+1.26%) ⬆️
wandb/sdk/internal/system/system_info.py 91.55% <0.00%> (+3.24%) ⬆️

@KyleGoyette KyleGoyette enabled auto-merge (squash) December 20, 2022 07:23
@KyleGoyette KyleGoyette merged commit 3a2adba into main Dec 20, 2022
@KyleGoyette KyleGoyette deleted the requirements-uri-support branch December 20, 2022 07:34
@kptkin kptkin changed the title fix(launch): Allow spaces in requirements file, remove duplicate wandb bootstrap file fix(launch): allow spaces in requirements file, remove duplicate wandb bootstrap file Jan 5, 2023
@kptkin kptkin added this to the sdk-2023-01.1 milestone Jan 5, 2023
@github-actions github-actions bot added cc-fix and removed cc-fix labels Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants