Fix patch behavior for trusses using python DX
#1337
Conversation
truss/remote/baseten/remote.py
Outdated
| def patch( | ||
| # TODO(nikhil): inspect source code rather than assume that watching an individual | ||
| # file indicates standalone models using the chains code gen framework | ||
| def _watch_path_requires_code_gen(self, watch_path: Path): |
There was a problem hiding this comment.
There's now 3 places where we have this magic to switch between traditional truss code and chains truss code. For V0 it's probably still ok, but I wonder if there's something better we can do via:
- scanning source code
- explicit command line flag propagated around
There was a problem hiding this comment.
Have you looked how watch for chains works? Can we somehow in the CLI branch of to using this instead of "hacking" this into here?
There was a problem hiding this comment.
I did poke around a bit to see if that would be possible, but decided against it for a couple small reasons:
- The existing
Watcherunderstandably has lots of chains specific stuff, and I think it would be some work to try and refactor what's not applicable to standalone models, fix error messages, etc - On the
cliside we could try and branch between existing logic and new logic around here, but then we'd likely have to duplicate some logic or introduce a decent amount of refactors toWatcherso the code can work for both cases
I think it might unfortunately be cleanest for now to drop this directly into the remote provider, how do you feel about that?
There was a problem hiding this comment.
The second commit has a slightly different approach, lmk if you prefer that
truss/remote/baseten/remote.py
Outdated
| # These imports are delayed, to handle pydantic v1 envs gracefully. | ||
| from truss_chains.deployment import code_gen | ||
|
|
||
| gen_truss_path = code_gen.gen_truss_model_from_source(patch_path) |
There was a problem hiding this comment.
What if it's a traditional truss model?
There was a problem hiding this comment.
Ah that's a different path on L414 above, but after discussing offline this will likely go away!
nnarayen
force-pushed
the
nikhil/fix-truss-watch-python-dx
branch
from
98c86e3 to
18d9e4a
Compare
| def _handle_intercepted_logs(logs: list[str], console: "rich_console.Console"): | ||
| if logs: | ||
| formatted_logs = textwrap.indent("\n".join(logs), " " * 4) | ||
| console.print(f"Intercepted logs from importing source code:\n{formatted_logs}") |
There was a problem hiding this comment.
This message (and in _handle_import_error below) are basically the same as before, but removed any explicit mention of chains. I don't think it detracts from the usefulness of the message, and otherwise we'd have to introduce another param to differentiate
| patcher = _ModelWatcher( | ||
| source=source, | ||
| model_name=model_name, | ||
| remote_provider=cast(b10_remote.BasetenRemote, remote_provider), |
There was a problem hiding this comment.
I don't love the explicit cast, but the interface method sync_truss_to_dev_version_by_name doesn't quite work for our use case with different directories for source / generated truss. Therefore, we need to do something similar to chainlets and peek into more low level patch API
truss/remote/baseten/remote.py
Outdated
| @@ -542,7 +542,7 @@ def do_patch(): | |||
| ), | |||
| ) | |||
|
|
|||
| def patch( | |||
| def patch_model( | |||
There was a problem hiding this comment.
Small naming change, since path_for_chainlet is explicit below
nnarayen
force-pushed
the
nikhil/fix-truss-watch-python-dx
branch
from
18d9e4a to
3598133
Compare
| @@ -29,6 +29,7 @@ | |||
| from truss.remote.baseten import custom_types as b10_types | |||
| from truss.remote.baseten import remote as b10_remote | |||
| from truss.remote.baseten import service as b10_service | |||
| from truss.remote.baseten.error import RemoteError | |||
There was a problem hiding this comment.
Nit: please follow the convetion in this file of importing modules (see also: https://google.github.io/styleguide/pyguide.html#22-imports)
🚀 What
This PR fixes the
truss watchbehavior for traditional trusses using the new chains DX. Use:💻 How
🔬 Testing
Tested by manually applying patches to a development model, confirmed that both changes to the model file and import files are reflected.