dagster icon indicating copy to clipboard operation
dagster copied to clipboard

Published 20 hours ago • verified publisher icon dagster-io

[DS][2/n] Update signature of AutoMaterializeRule

Open OwenKephart opened this issue 9 months ago • 2 comments

Summary & Motivation

Continues work from the PR below, just banishing more traces of the AssetConditionEvaluationContext

How I Tested These Changes

OwenKephart avatar Apr 29 '24 22:04 OwenKephart

  • #21511 Graphite: 2 dependent PRs (#21512 Graphite, #21520 Graphite)
  • #21510 Graphite
  • #21508 Graphite
  • #21507 Graphite
  • #21505 Graphite
  • #21504 Graphite
  • #21503 Graphite
  • #21502 Graphite
  • #21501 Graphite
  • #21500 Graphite 👈
  • #21499 Graphite
  • master

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

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

OwenKephart avatar Apr 29 '24 22:04 OwenKephart

Would have been helpful to have in PR summary a line like

"Changes all function signatures to take SchedulingConditionEvaluationContext instead of AssetConditionEvaluationContext and accesses the old AssetConditionEvaluationContext by accessing the legacy_context property in the body of all functions"

That way I could have known this was a super mechnical rename and effectively not had to review the actual code at all.

schrockn avatar May 02 '24 13:05 schrockn

Merge activity

  • May 3, 5:31 PM EDT: @OwenKephart started a stack merge that includes this pull request via Graphite.
  • May 3, 5:34 PM EDT: Graphite rebased this pull request as part of a merge.
  • May 3, 5:35 PM EDT: @OwenKephart merged this pull request with Graphite.

OwenKephart avatar May 03 '24 21:05 OwenKephart