Working mirror tested only with filesystem mirror #261
Conversation
src/worker/lambda/handlerPuller.go
Outdated
There was a problem hiding this comment.
What are you trying to accomplish with the changes to handlerPuller.go? Do we need to do anything here if we only support mirrors over HTTP?
There was a problem hiding this comment.
Only a small change just to copy the requirements.txt into the target dir, because without copying it any installs with requirements.txt would be ignored as it would not be seen by packagepuller.go
| def f(event): | ||
| pkg = event["pkg"] | ||
| alreadyInstalled = event["alreadyInstalled"] | ||
| pip_mirror = event.get("pip_mirror", "") |
There was a problem hiding this comment.
If we only call this from one place and we also pass it, do we need to make it optional?
There was a problem hiding this comment.
You are right there will always be the key provided. If the user doesnt have a mirror listed in the config then the string will be empty, this change will be reflected in the next commit
| def f(event): | ||
| pkg = event["pkg"] | ||
| alreadyInstalled = event["alreadyInstalled"] | ||
| pip_mirror = event.get("pip_mirror", "") | ||
| if not alreadyInstalled: | ||
| try: |
There was a problem hiding this comment.
There's a lot of code in the try, but it only catches exceptions from check_output. Let's have a single check_output in the try block. For that matter, let's have a single check_output in general. We can pass in a cmd variable (or similar) that gets configured differently.
There was a problem hiding this comment.
Totally agree. Let me know if the code in the next commit looks better.
| host_start_index = pip_mirror.find('://') + 3 | ||
| host_end_index = pip_mirror.find('/', host_start_index) | ||
| mirror_host = pip_mirror[host_start_index:host_end_index] | ||
| cmds = ['pip3', 'install', '--no-deps', pkg, '--cache-dir', '/tmp/.cache', '-t', '/host/files', f"--trusted-host={mirror_host}", f"--index-url={pip_mirror}", "-vvv"] |
There was a problem hiding this comment.
We should avoid copying the original args then adding. If we need to make future changes, that will have to happen in two places. Better to have the basic args, then if pip_mirror is set, append the additional ones.
| host_end_index = pip_mirror.find('/', host_start_index) | ||
| mirror_host = pip_mirror[host_start_index:host_end_index] | ||
| cmds = ['pip3', 'install', '--no-deps', pkg, '--cache-dir', '/tmp/.cache', '-t', '/host/files', f"--trusted-host={mirror_host}", f"--index-url={pip_mirror}", "-vvv"] | ||
| print(f"[packaagePullerInstaller.py] attempting install with command: {cmds}") |
| ### Potential Issues | ||
|
|
||
| If you run into an error it will likely present as unable to find | ||
| /host/files. This is because the install failed leading to the target |
There was a problem hiding this comment.
Do we need to mess with /host/files if we only support mirrors over HTTP?
There was a problem hiding this comment.
That part of the readme is in reference to the allocation of the unpacked .whl/ .tar.gz files after installing from the index which are then installed there. So its not restricted to the type of mirror, say a mirror hosted locally would run into the same issues.
| configuration file you can run `bandersnatch mirror` once more to | ||
| begin installation. This may take several minutes. | ||
|
|
||
| ### Nginx Support |
There was a problem hiding this comment.
Are there shorter directions we can give for nginx? E.g., is there a one-liner we can use to start a container with nginx? https://hub.docker.com/_/nginx
|
|
||
| For documentation on how to set up the mirror use: [bandersnatch docs](https://bandersnatch.readthedocs.io/en/latest/mirror_configuration.html) | ||
|
|
||
| It is important to note that if you do not use an allowlist or |
There was a problem hiding this comment.
Should we give an example of the allowlist?
There was a problem hiding this comment.
I think that would be helpful. I have included one in the next commit.
docs/worker/pip-mirror.md
Outdated
| @@ -0,0 +1,176 @@ | |||
| # Installing a Pip Mirror | |||
|
|
|||
| When running python based lambdas in OpenLambda packages | |||
There was a problem hiding this comment.
Hyphenate: "Python-based lambdas"
Edited packagePuller.go, handlerPuller.go and packagePullerInstaller.py
packagePuller.go:
Added pip_mirror configuration field to message sent to sandbox. Allows packagePullerInstaller.py to see if there is a pip mirror installed or not, and if so where.
handlerPuller.go:
Was not originally pulling the requirements.txt file from the registry to the targetDir. This caused any requested downloads with requirements.txt to be ignored and not ran with or without a pip mirror indicated as set up.
packagePullerInstaller.py:
Changed so that if a destination for a backup mirror was specified, pip then installs only from the mirror with --index-url. This supports a web server or docker implementation for storing the mirror.