add default configs for all served runnables #209

jakerachleff · 2023-11-10T23:04:34Z

Configures the run name to be the path, and some information about the associated repo to be in the metadata of the traced run

eyurtsev

looks

eyurtsev · 2023-11-11T02:39:26Z

langserve/server.py

@@ -554,6 +562,38 @@ def _route_name_with_config(name: str) -> str:
    InvokeResponse = create_invoke_response_model(model_namespace, output_type_)
    BatchResponse = create_batch_response_model(model_namespace, output_type_)

+    def _update_config_with_defaults(incomingConfig: RunnableConfig) -> RunnableConfig:


Could we move it into a standalone function?

eyurtsev · 2023-11-11T02:40:51Z

langserve/server.py

+        is_hosted = os.environ.get("HOSTED_LANGSERVE_ENABLED", "false").lower() == "true"
+        if is_hosted:
+            hosted_metadata = {
+                "Commit SHA": os.environ.get("HOSTED_LANGSERVE_GIT_COMMIT", ""),


Should the keys follow a particular format for rendering purposes / name collision purposes?

I'm not sure, I started a thread internally to discuss the rendering questions. My worry on naming collision proof names is that they will be hard to read and thus hard to filter on

eyurtsev · 2023-11-11T02:41:03Z

langserve/server.py

@@ -684,6 +725,7 @@ async def batch(
            {k: v for k, v in config_.items() if k in config_keys}
            for config_ in get_config_list(configs, len(inputs_))
        ]
+        print(configs_)


Suggested change

print(configs_)

eyurtsev · 2023-11-11T02:42:32Z

This looks pretty reasonable to me

eyurtsev · 2023-11-13T15:40:25Z

langserve/server.py

@@ -122,6 +123,45 @@ def _unpack_request_config(
    )


+def _update_config_with_defaults(
+    path: str, incomingConfig: RunnableConfig, endpoint: Optional[str] = None


Suggested change

path: str, incomingConfig: RunnableConfig, endpoint: Optional[str] = None

path: str, incomingConfig: RunnableConfig, *, endpoint: Optional[str] = None

eyurtsev · 2023-11-13T15:41:21Z

langserve/server.py

+    is_hosted = os.environ.get("HOSTED_LANGSERVE_ENABLED", "false").lower() == "true"
+    if is_hosted:
+        hosted_metadata = {
+            "Commit SHA": os.environ.get("HOSTED_LANGSERVE_GIT_COMMIT", ""),


The naming convention for these is different from langserve version etc. Should we use the same style? (I don't care which style is used)

what is the current convention?

just __langserve?

yeah but whatever you prefer -- i just chose arbitrarily

langserve/server.py

eyurtsev · 2023-11-13T15:41:46Z

langserve/server.py

@@ -735,7 +787,7 @@ async def stream(
        err_event = {}
        validation_exception: Optional[BaseException] = None
        try:
-            config, input_ = await _get_config_and_input(request, config_hash)
+            config, input_ = await _get_config_and_input(request, config_hash, "stream")


Suggested change

config, input_ = await _get_config_and_input(request, config_hash, "stream")

config, input_ = await _get_config_and_input(request, config_hash, endpoint="stream")

eyurtsev · 2023-11-13T15:43:23Z

langserve/server.py

@@ -474,6 +515,11 @@ def add_routes(
            f"If specifying path please start it with a `/`"
        )

+    if "run_name" in config_keys:
+        raise ValueError(
+            "Cannot configure run_name. Please remove it from config_keys."


Do we care about key collisions arising from identical path on 2 different API routers? (I think it's fine actually)

oh this is a good point... we probably should?

We derive it from request.url.path and always get the config based on the request?

might limit future support for complex paths tho

I'm OK with whatever you choose here -- just wanted to surface that collisions could potentially appear

Co-authored-by: Eugene Yurtsev <eyurtsev@gmail.com>

eyurtsev · 2023-11-20T21:52:40Z

langserve/server.py

@@ -132,6 +133,56 @@ def _unpack_request_config(
    )


+def _update_config_with_defaults(
+    path: str,
+    incomingConfig: RunnableConfig,


Suggested change

incomingConfig: RunnableConfig,

incoming_config: RunnableConfig,

(python naming convention)

eyurtsev · 2023-11-20T21:54:44Z

langserve/server.py

@@ -794,7 +840,9 @@ async def stream(
        err_event = {}
        validation_exception: Optional[BaseException] = None
        try:
-            config, input_ = await _get_config_and_input(request, config_hash)
+            config, input_ = await _get_config_and_input(
+                request, config_hash, endpoint="stream"


are we missing a similar config override for stream_log endpoint?

This reverts commit b60b52d.

cla-bot bot added the cla-signed label Nov 10, 2023

eyurtsev reviewed Nov 11, 2023

View reviewed changes

jakerachleff requested a review from eyurtsev November 13, 2023 02:47

eyurtsev approved these changes Nov 13, 2023

View reviewed changes

jakerachleff requested a review from eyurtsev November 16, 2023 23:23

jakerachleff added 4 commits November 20, 2023 11:44

add default configs for all served runnables

be6e972

fix all endpoints

017fc78

working

2fa4b5e

address comments

1ee54e8

jakerachleff force-pushed the 2023-11-10-format-langsmith-runs branch from 30cb986 to 1ee54e8 Compare November 20, 2023 21:45

jakerachleff and others added 2 commits November 20, 2023 13:50

fix issues

9e132c0

Update langserve/server.py

7ab3c8e

Co-authored-by: Eugene Yurtsev <eyurtsev@gmail.com>

eyurtsev reviewed Nov 20, 2023

View reviewed changes

jakerachleff merged commit b60b52d into main Nov 20, 2023

jakerachleff deleted the 2023-11-10-format-langsmith-runs branch November 20, 2023 21:59

jakerachleff added a commit that referenced this pull request Nov 20, 2023

Revert "add default configs for all served runnables (#209)"

bfad05a

This reverts commit b60b52d.

jakerachleff mentioned this pull request Nov 20, 2023

Revert "add default configs for all served runnables" #248

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add default configs for all served runnables #209

add default configs for all served runnables #209

jakerachleff commented Nov 10, 2023 •

edited

Loading

eyurtsev left a comment

eyurtsev Nov 11, 2023

eyurtsev Nov 11, 2023

jakerachleff Nov 13, 2023

eyurtsev Nov 11, 2023

eyurtsev commented Nov 11, 2023

eyurtsev Nov 13, 2023

eyurtsev Nov 13, 2023

jakerachleff Nov 16, 2023

jakerachleff Nov 16, 2023

eyurtsev Nov 17, 2023

eyurtsev Nov 13, 2023

eyurtsev Nov 13, 2023

jakerachleff Nov 16, 2023

jakerachleff Nov 16, 2023

jakerachleff Nov 16, 2023

eyurtsev Nov 17, 2023

eyurtsev Nov 20, 2023

eyurtsev Nov 20, 2023

eyurtsev Nov 20, 2023

	path: str, incomingConfig: RunnableConfig, endpoint: Optional[str] = None
	path: str, incomingConfig: RunnableConfig, *, endpoint: Optional[str] = None

	config, input_ = await _get_config_and_input(request, config_hash, "stream")
	config, input_ = await _get_config_and_input(request, config_hash, endpoint="stream")

	incomingConfig: RunnableConfig,
	incoming_config: RunnableConfig,

add default configs for all served runnables #209

add default configs for all served runnables #209

Conversation

jakerachleff commented Nov 10, 2023 • edited Loading

eyurtsev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eyurtsev commented Nov 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakerachleff commented Nov 10, 2023 •

edited

Loading