dagster icon indicating copy to clipboard operation
dagster copied to clipboard

Use usable as resource for K8sPipeClient

Open schrockn opened this issue 9 months ago • 1 comments

Summary & Motivation

How I Tested These Changes

Screenshot 2024-05-08 at 6.54.54 AM.png

schrockn avatar May 08 '24 10:05 schrockn

  • #21724 Graphite
  • #21719 Graphite 👈
  • #21718 Graphite
  • master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @schrockn and the rest of your teammates on Graphite Graphite

schrockn avatar May 08 '24 10:05 schrockn

UsableAsResourceParam probably more explicit and correct name. But went for shorter name.

schrockn avatar May 08 '24 15:05 schrockn

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-kk2cbpekw-elementl.vercel.app https://usable-as-resource-2.dagster.dagster-docs.io

Direct link to changed pages:

github-actions[bot] avatar May 08 '24 15:05 github-actions[bot]

I was wondering about this - thanks @schrockn .

This change, https://github.com/dagster-io/dagster/pull/21715/files, can now be reverted which included the _PipesSubprocess class in Sphinx.

cmpadden avatar May 08 '24 16:05 cmpadden

We may also want to consider these references:

python_modules/libraries/dagster-aws/dagster_aws/pipes.py
245:PipesLambdaClient = ResourceParam[_PipesLambdaClient]

python_modules/libraries/dagster-k8s/dagster_k8s/pipes.py
428:PipesK8sClient = ResourceParam[_PipesK8sClient]

python_modules/libraries/dagster-databricks/dagster_databricks/pipes.py
210:PipesDatabricksClient = ResourceParam[_PipesDatabricksClient]

python_modules/dagster/dagster_tests/execution_tests/pipes_tests/in_process_client.py
129:InProcessPipesClient = ResourceParam[_InProcessPipesClient]

python_modules/libraries/dagster-docker/dagster_docker/pipes.py
218:PipesDockerClient = ResourceParam[_PipesDockerClient]

cmpadden avatar May 08 '24 16:05 cmpadden

Yes will do that in follow up once we finalize naming

On Wed, May 8 2024 at 12:18 PM, colton < @.*** > wrote:

We may also want to consider these references:

python_modules/libraries/dagster-aws/dagster_aws/pipes.py 245:PipesLambdaClient = ResourceParam[_PipesLambdaClient]

python_modules/libraries/dagster-k8s/dagster_k8s/pipes.py 428:PipesK8sClient = ResourceParam[_PipesK8sClient]

python_modules/libraries/dagster-databricks/dagster_databricks/pipes.py 210:PipesDatabricksClient = ResourceParam[_PipesDatabricksClient]

python_modules/dagster/dagster_tests/execution_tests/pipes_tests/in_process_client.py 129:InProcessPipesClient = ResourceParam[_InProcessPipesClient]

python_modules/libraries/dagster-docker/dagster_docker/pipes.py 218:PipesDockerClient = ResourceParam[_PipesDockerClient]

— Reply to this email directly, view it on GitHub ( https://github.com/dagster-io/dagster/pull/21719#issuecomment-2100939228 ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/AG3IK6NG6R2M76WG7GPVUQLZBJF4VAVCNFSM6AAAAABHMXCLBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBQHEZTSMRSHA ). You are receiving this because you were mentioned. Message ID: <dagster-io/dagster/pull/21719/c2100939228 @ github. com>

schrockn avatar May 08 '24 16:05 schrockn

Changed to UsableAsResourceParam

schrockn avatar May 08 '24 16:05 schrockn

Would like @benpankow's take at a minimum on naming here until I merge

schrockn avatar May 08 '24 17:05 schrockn

Merge activity

  • May 8, 7:51 PM EDT: @schrockn started a stack merge that includes this pull request via Graphite.
  • May 8, 7:54 PM EDT: The Graphite merge of this pull request was cancelled.
  • May 8, 8:32 PM EDT: @schrockn started a stack merge that includes this pull request via Graphite.
  • May 8, 8:37 PM EDT: Graphite rebased this pull request as part of a merge.
  • May 8, 8:38 PM EDT: @schrockn merged this pull request with Graphite.

schrockn avatar May 08 '24 23:05 schrockn