ploomber / soopervisor

☁️ Export Ploomber pipelines to Kubernetes (Argo), Airflow, AWS Batch, SLURM, and Kubeflow.
https://soopervisor.readthedocs.io
Apache License 2.0
45 stars 18 forks source link

bundle custom lib in docker image #87

Closed Wxl19980214 closed 2 years ago

Wxl19980214 commented 2 years ago

Describe your changes

Add path check. If Path(lib/') exist, add a PYTHONPATH in docerfile to look for packages under lib.

Issue ticket number and link

Closes #85

Checklist before requesting a review

edublancas commented 2 years ago

overall, it looks good!

did you get a chance to test it?

Essentially, we wanna make sure that if I have a lib/something.py, I can do:

python

and then:

import something
edublancas commented 2 years ago

please write an automated test, see this for details: https://github.com/ploomber/soopervisor/issues/88

Wxl19980214 commented 2 years ago
@edublancas  Yes. I have tested in this way. This is my project structure:
project/
        -> lib/ 
                  ->package_a.py (contains function printTest() )
        -> fit.py  (from lib.package_a import printTest )
        -> pipeline.yaml
        -> soopervisor.yaml
        ....

And I use sooperviosr add and export to build the image. It looks like there's no error when I run the image. 
idomic commented 2 years ago

@Wxl19980214 please add a few tests.

Wxl19980214 commented 2 years ago

@edublancas @idomic I am having troubling writing test: Here is what I have:

@pytest.mark.parametrize('CLASS_', [
    AWSBatchExporter,
    AirflowExporter,
    ArgoWorkflowsExporter,
    KubeflowExporter,
],
                         ids=[
                             'batch',
                             'airflow',
                             'argo',
                             'kubeflow',
                         ])
def test_docker_local_lib_import(
    mock_docker_and_batch,
    tmp_sample_project,
    no_sys_modules_cache,
    skip_repo_validation,
    capsys,
    CLASS_,
):

    exporter = CLASS_.new(path_to_config='soopervisor.yaml', env_name='serve')

    exporter.add()
    exporter.export(mode='force', skip_tests=True)

    client = docker.from_env()
    client.containers.run('python', 'import package_a')

    captured = capsys.readouterr()

    print(captured.out)

I have added the lib/ directory with package_a under asset/sample_project. However, it seems like the docker.from_env() is mocked so I cannot get real result. Any ideas?

Wxl19980214 commented 2 years ago

Also for some reason, i cannot recreate what I had last week. After soopervisor export, what command do we use to run the image? docker run example:latest? I have tried and it looks like not working

idomic commented 2 years ago

Maybe don't mock docker.from_env() ? I don't see a way around it if you'd like to check what's inside the container and run commands in it.

You can run the image interactively with the -it flag, is that what you're talking about? You can also use the commander and run something like this:

e.run('docker',
              'run',
              image_local,
              'ploomber',
              'status',
              '--entry-point',
              entry_point,
              description='Testing image',
              error_message='Error while testing your docker image with')
Wxl19980214 commented 2 years ago

I mean after soopervisor export argo --skip-tests --ignore-git --mode force I should get a docker image. Last week I can run some command to actually run this image, and in there I can directly test by python, import package_a. But today it looks like I cannot activate image.

idomic commented 2 years ago

That's right. After the export command you should have the image tagged locally and you can simply use docker run tag -it to run it and debug. You can also list the available images to see if it's not there for some reason.

Wxl19980214 commented 2 years ago

Ok this is weird, I have checked I have images locally by docker images

example $ docker images
REPOSITORY TAG IMAGE ID CREATED SIZE example latest af4bedf7589c 2 hours ago 1.15GB

but when I do: docker run example -it, it returns this: [FATAL tini (6)] exec -it failed: No such file or directory

idomic commented 2 years ago

It should be something like this docker run image_name:tag_name

On Mon, Jun 27, 2022 at 2:35 PM Xilin @.***> wrote:

Ok this is weird, I have checked I have images locally by docker images

example $ docker images REPOSITORY TAG IMAGE ID CREATED SIZE example latest af4bedf7589c 2 hours ago 1.15GB

but when I do: docker run example -it, it returns this: [FATAL tini (6)] exec -it failed: No such file or directory

— Reply to this email directly, view it on GitHub https://github.com/ploomber/soopervisor/pull/87#issuecomment-1167731477, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACYPJOJP35GOVK2U3PWQN2LVRHX5LANCNFSM5ZYUI6SA . You are receiving this because you were mentioned.Message ID: @.***>

Wxl19980214 commented 2 years ago

docer run example:latest? It just returns nothing and not actually running the image.

idomic commented 2 years ago

Issue solved.

edublancas commented 2 years ago

so tests are passing now, I feel like we should tackle https://github.com/ploomber/soopervisor/issues/88 now to ensure this is working and also to have some recipe for future tests that involve the docker image.

thoughts? @idomic

Wxl19980214 commented 2 years ago

@edublancas I think he meant local test are done. I am still figuring out a way to write automated tests.

edublancas commented 2 years ago

ok cool

Wxl19980214 commented 2 years ago

@edublancas We have a PR for this branch already. I have commented in the text_export.py file.

edublancas commented 2 years ago

is this ready for review?

Wxl19980214 commented 2 years ago

@edublancas Yes. I couldn't find where the changes were requested. I thought I resolved them all.

idomic commented 2 years ago

@Wxl19980214 Did you do some manual tests as well?

Wxl19980214 commented 2 years ago

@idomic Yes I have tried with argo and kubernate. I can go ahead and check other backend as well

idomic commented 2 years ago

Please test from main with another backend let's say batch

Wxl19980214 commented 2 years ago

Just tested. There is no issue.