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

[dagster-airlift] Python operator docs #26096

Open
wants to merge 1 commit into
base: dpeng817/k8s_docs
Choose a base branch
from

Conversation

dpeng817
Copy link
Contributor

Summary & Motivation

How I Tested These Changes

Changelog

Insert changelog entry or delete this section.

@dpeng817 dpeng817 marked this pull request as ready for review November 22, 2024 03:53
@graphite-app graphite-app bot added the docs-to-migrate Docs to migrate to new docs site label Nov 22, 2024
@dpeng817 dpeng817 force-pushed the dpeng817/python_operator_docs branch from 4c6eb2c to 1e187a5 Compare November 22, 2024 03:54
Copy link

graphite-app bot commented Nov 22, 2024

Graphite Automations

"Add a 'docs-to-migrate' label to PRs with docs" took an action on this PR • (11/22/24)

1 label was added to this PR based on Christopher DeCarolis's automation.

def script_result(context: AssetExecutionContext):
return (
PipesSubprocessClient()
.run(
Copy link

Choose a reason for hiding this comment

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

context.op_execution_context is a property that doesn't exist on AssetExecutionContext. To get the OpExecutionContext that PipesSubprocessClient requires, use context.get_op_execution_context() instead.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@dpeng817 dpeng817 force-pushed the dpeng817/python_operator_docs branch from 267d5ec to 7f90cc5 Compare November 24, 2024 04:57
def script_result(context: AssetExecutionContext):
return (
PipesSubprocessClient()
.run(context=context, command="python /path/to/script.py")
Copy link

Choose a reason for hiding this comment

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

The context parameter should be passed as context=context.op_execution_context since PipesSubprocessClient.run() expects an OpExecutionContext. The current code passes an AssetExecutionContext which will cause type errors. This can be fixed by accessing the underlying op_execution_context property.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@dpeng817 dpeng817 force-pushed the dpeng817/python_operator_docs branch 2 times, most recently from e7d0535 to b84bce9 Compare November 24, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-to-migrate Docs to migrate to new docs site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant