Happy path fix by peadaroh · Pull Request #48 · humanloop/humanloop-python

peadaroh · 2025-02-28T16:50:16Z

No description provided.

…cel run on program abort

peadaroh · 2025-03-03T10:43:22Z

src/humanloop/eval_utils/run.py

        except Exception as e:
-            logger.error(f"Failed to log: {e}")
+            error_message = str(e).replace("\n", " ")
+            if len(error_message) > 100:


This seems hacky, is it really needed?

Noticed 502s tend to return a full HTML, polluting the screen - so I've condensed the error by removing the newlines and trimmed the length

peadaroh · 2025-03-03T10:44:03Z

src/humanloop/eval_utils/run.py

        # Notify the run_eval utility about one Log being created
        if log_belongs_to_evaluated_file(log_args=kwargs):
            evaluation_context = get_evaluation_context()
+            evaluation_context.logged = True


is this for the specific datapoint and evaluation run?

Yes - every datapoint is ran in isolation from other threads, so only one log statement is accepted per evaluated file - run - datapoint

peadaroh · 2025-03-03T10:44:44Z

src/humanloop/eval_utils/run.py

        # Running the evaluation locally
-        logger.info(
-            f"{CYAN}\nRunning '{hl_file.name}' over the Dataset '{hl_dataset.name}' using {workers} workers{RESET} "
+        sys.stdout.write(


why sys vs logger?

logger doesn't interact with ANSI codes such as color, move cursor out of the box - will look into whether the logger can accept those

peadaroh · 2025-03-03T10:45:13Z

src/humanloop/eval_utils/run.py

-                logger.warning(
-                    msg=f"\nYour {hl_file.type}'s `callable` failed for Datapoint: {dp.id}. \n Error: {str(e)}"
-                )
+                error_message = str(e).replace("\n", " ")


this logic was repeated above, why is it needed?

peadaroh · 2025-03-03T10:45:38Z

src/humanloop/eval_utils/run.py

+                    )

        with ThreadPoolExecutor(max_workers=workers) as executor:
+            futures = []


could you explain the move to futures?

was investigating some hanging threads - reverting the change

peadaroh · 2025-03-03T10:46:10Z

src/humanloop/eval_utils/run.py

    def increment(self):
        """Increment the progress bar by one finished task."""
        with self._lock:
+            # NOTE: There is a deadlock here that needs further investigation


what does this deadlock cause? how important is it to resolve?

can no longer reproduce it now - will remove the comment

peadaroh · 2025-03-03T10:46:37Z

src/humanloop/eval_utils/run.py

    # Wait for the Evaluation to complete then print the results
    complete = False
+
+    wrote_explainer = False


Weirdly named variable, i have no idea what this could be for fromthe name

peadaroh · 2025-03-03T10:47:48Z

src/humanloop/eval_utils/run.py

-                id=local_evaluator.id,
-                start_time=start_time,
-                end_time=datetime.now(),
+                log_dict = log


in general assignment like this may cause issues

fern side issue: LogResponse is polymorphic on the backend, so it's likely not parsing the json into a pedantic object

peadaroh · 2025-03-03T10:48:07Z

src/humanloop/eval_utils/run.py

+                    end_time=datetime.now(),
+                )
+                error_message = str(e).replace("\n", " ")
+                if len(error_message) > 100:


this logic is repeated twice above, why is it needed?

peadaroh · 2025-03-03T10:48:50Z

src/humanloop/flows/client.py

            raise ApiError(status_code=_response.status_code, body=_response.text)
        raise ApiError(status_code=_response.status_code, body=_response_json)

+    def update_log(


Was surprised by this change in the diff.

Was this just added?

This was a separate change from reordering the update_log path to be under/right after creating a flow log. Just part of fern autogenerating SDK; quick win came from talking w Jordan and Robin about agents DX last week.

not sure - reverted the file to the version on main branch, works as expected - branch likely started before updating some docstrings on response models last week

peadaroh · 2025-03-03T10:49:21Z

src/humanloop/otel/exporter.py

        self._shutdown = True
        for thread in self._threads:
-            thread.join()
+            thread.join(timeout=5000 / 1000)


5000/1000 - this is weird

jamesbaskerville · 2025-03-03T13:06:22Z

src/humanloop/otel/processor/prompts.py

@@ -1,4 +1,4 @@
-import deepdiff
+import deepdiff  # type: ignore [import]


is this necessary? or did we lose types from the version downgrade?

good point - the downgrade is not required. reverted deepdiff to higher version, removing the need for the ignore

jamesbaskerville · 2025-03-03T13:08:32Z

src/humanloop/utilities/flow.py

-                        func=func,
-                        output=None,
-                    )
+                    output_stringified = None


is this change ok? what's the purpose?

yes - output should be none when error is thrown, otherwise the backend complains we provided two output properties on a flow log (this constraint was relaxed in the /otel PR that's not merged yet)

EDIT: seems the reverse did not solve the error, I likely pushed the import deepdiff line without checking the lining previously

jamesbaskerville

generally lgtm

fern-api bot and others added 4 commits February 28, 2025 15:25

Release 0.8.25

1739407

Fix the path where a normal callable is used

564e2b7

deduplicate logging

93f1a6d

Colors on print messages, hanging threads, allow no type in file, can…

f8f27e8

…cel run on program abort

peadaroh commented Mar 3, 2025

View reviewed changes

Andrei Bratu and others added 3 commits March 3, 2025 12:22

fix output on @flow error, @flow + logging inside edge case on evals

3963f52

Don't query if no local evaluators; cleanup error printing

3c977cf

mypy complains about no type hints

9ad1a6f

jamesbaskerville reviewed Mar 3, 2025

View reviewed changes

jamesbaskerville approved these changes Mar 3, 2025

View reviewed changes

PR feedback

3335c35

andreibratu merged commit 5898e74 into master Mar 3, 2025
7 checks passed

		@@ -1,4 +1,4 @@
		import deepdiff
		import deepdiff # type: ignore [import]

Conversation

peadaroh commented Feb 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesbaskerville Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreibratu Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamesbaskerville left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jamesbaskerville Mar 3, 2025 •

edited

Loading

andreibratu Mar 3, 2025 •

edited

Loading