Introduce forward-only plans#109
Conversation
izeigerman
requested review from
crericha,
eakmanrq,
georgesittas,
tobymao and
vchan
izeigerman
force-pushed
the
forward-only-plan
branch
from
abd9d6f to
dc19c58
Compare
| and upstream_snapshot.is_paused | ||
| ): | ||
| raise PlanError( | ||
| f"Modified model '{model_name}' depends on a paused forward-only snapshot {upstream_snapshot.snapshot_id}. " |
There was a problem hiding this comment.
Should we show the forward-only model name instead of snapshot id? I believe this is currently the only place where we tell users about the concept of snapshots.
There was a problem hiding this comment.
This is a good point. The challenge here is that we don't talk about just any version of that parent model, but a very specific instance of it. How do you think we can covey this without exposing a snapshot?
There was a problem hiding this comment.
I can use the name here but then still use a snapshot id when I say "2) promote the snapshot ...".
There was a problem hiding this comment.
or I guess I might just drop option 2) from suggestions
There was a problem hiding this comment.
I think option 2) is useful. How hard would it be to say something like "model that is currently active in these environments: needs to be promoted to production environment". Basically check where all that snapshot is active in. In most cases it should be a single environment.
There was a problem hiding this comment.
Ah, you helped me realize that we can refer to it as just "the current version" since we know that this snapshot is currently in the user's local state. How about this then:
raise PlanError(
f"Modified model '{model_name}' depends on the paused version of model '{upstream}'. "
"Possible remedies: "
"1) make sure your codebase is up-to-date; "
f"2) promote the current version of model '{upstream}' in the production environment; "
"3) recreate this plan in a forward-only mode."
)
There was a problem hiding this comment.
Yeah that works. I think for this person, especially if they are less technical, it might be difficult to understand how to promote just that upstream model to product but it is likely in that case they will be pinging an engineer anyways to resolve the issue. Also telling them the environment helps some but not completely since in this scenario it is likely someone else's environment and they are pinging them anyways. So good for now.
There was a problem hiding this comment.
Thank you, pushed
3 participants
This PR pretty much covers the entire forward-only functionality. Here's what remains and will be covered in subsequent PRs: