Skip to content

Guard against wrong dagger binary in Python SDK#3310

Merged
helderco merged 3 commits into
dagger:cloakfrom
gerhard:guard-against-wrong-dagger-in-python-sdk
Oct 12, 2022
Merged

Guard against wrong dagger binary in Python SDK#3310
helderco merged 3 commits into
dagger:cloakfrom
gerhard:guard-against-wrong-dagger-in-python-sdk

Conversation

@gerhard
Copy link
Copy Markdown
Contributor

@gerhard gerhard commented Oct 11, 2022

We want to fail if the "wrong" dagger is used. Versions prior to 0.3.x will not have commands that these SDKs depend on.

Follow-up to #3285
Part of #3176

@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 11, 2022

👷 Deploy request for cloak-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 56a34ba

@gerhard
Copy link
Copy Markdown
Contributor Author

gerhard commented Oct 11, 2022

@samalba @helderco How do I test this manually?

examples/python/client-simple does not seem to work as captured in the README:

image

@helderco
Copy link
Copy Markdown
Contributor

helderco commented Oct 11, 2022

Yes, looks like you're activating the environment on a subshell but then install and run on the global environment. I'll take a look at those instructions tomorrow. For now try moving the install and run inside those parenthesis.

Don't forget to update paths accordingly because of the cd.

@helderco
Copy link
Copy Markdown
Contributor

Try this:

python3 -m venv ../../.venv
source ../../venv/bin/activate
pip3 install -r requirements.txt
python3 query.py

@helderco
Copy link
Copy Markdown
Contributor

We'll also need to add a name to cloak.yaml.

@gerhard
Copy link
Copy Markdown
Contributor Author

gerhard commented Oct 11, 2022

All working, thanks @helderco!

Please review & merge if it looks good to you.

Screenshot 2022-10-11 at 21 31 30

@gerhard gerhard marked this pull request as ready for review October 11, 2022 20:36
@gerhard gerhard requested review from helderco and samalba October 11, 2022 20:37
@gerhard
Copy link
Copy Markdown
Contributor Author

gerhard commented Oct 11, 2022

We may want to do something similar for the Python SDK: #3292

Leaving it with you @helderco @samalba

Copy link
Copy Markdown
Contributor

@helderco helderco left a comment

Choose a reason for hiding this comment

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

👍

Comment thread examples/python/client-simple/README.md Outdated
Comment thread .gitignore Outdated
Comment thread sdk/python/dagger/src/dagger/engine.py Outdated
@helderco
Copy link
Copy Markdown
Contributor

We may want to do something similar for the Python SDK: #3292

Yes I have that on my list but am prioritizing getting a codegen'ed client first. I'll create an issue.

Gerhard Lazu added 3 commits October 12, 2022 12:07
Signed-off-by: Gerhard Lazu <gerhard@dagger.io>
This is meant to guard against running the "wrong" dagger. Versions
prior to 0.3.x will not have commands that these SDKs depend on.

Why exit 127 instead of 1?
https://tldp.org/LDP/abs/html/exitcodes.html

Signed-off-by: Gerhard Lazu <gerhard@dagger.io>
.venv (instead of venv) seems to be a Python convention.

Signed-off-by: Gerhard Lazu <gerhard@dagger.io>
@gerhard
Copy link
Copy Markdown
Contributor Author

gerhard commented Oct 12, 2022

@helderco please merge if this looks good to you.

@helderco helderco merged commit 880cb8d into dagger:cloak Oct 12, 2022
@gerhard gerhard deleted the guard-against-wrong-dagger-in-python-sdk branch October 12, 2022 12:03
This was referenced Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants