Fix package loading in Python runner by litvand · Pull Request #113 · hashintel/hash

litvand · 2021-12-20T10:45:28Z

🌟 What is the purpose of this PR?

Python init PR #2
Previous PR: #112

🔗 Related links

Asana task description

🔍 What does this change?

Ignores packages' owned fields temporarily
Fixes sending runner errors to Rust (using nng)
Fixes paths where packages are expected to be
Fixes hash_util import in packages

🛡 Tests

✅ Manual Tests

❓ How to test this?

Run any experiment.
Check that Python reaches behavior execution or messages package init

lgtm-com · 2021-12-20T10:55:13Z

This pull request introduces 1 alert when merging bf51440 into 67a8cd8 - view on LGTM.com

new alerts:

1 for Unused import

Alfred-Mountfield

Blocking merging until #112 is merged and this branch is rebased for easier review

CLAassistant · 2022-01-06T14:57:09Z

All committers have signed the CLA.

lgtm-com · 2022-01-06T15:02:55Z

This pull request introduces 1 alert when merging 57f5f6118c37ee96e795ca23c0ddd5bb06c9c637 into f7bf804 - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2022-01-06T15:21:47Z

This pull request introduces 1 alert when merging 3a8b19179da84e6b4352224467e15bda607b0d45 into b8274df - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2022-01-06T15:31:44Z

This pull request introduces 1 alert when merging b378392 into b8274df - view on LGTM.com

new alerts:

1 for Unused import

Alfred-Mountfield · 2022-01-06T17:21:38Z



+# TODO(permanently?): Keep in sync with PACKAGE_TYPE
+def str_from_pkg_type(t):


Can we avoid single char variable names if possible?

Alfred-Mountfield · 2022-01-06T17:23:16Z

 def get_pkg_path(pkg_name, pkg_type):
-    return Path("../../../simulation/package/{}/packages/{}/package.py".format(
+    # The engine should be started from the engine's root directory in the repo.
+    return Path("./src/simulation/package/{}/packages/{}/package.py".format(


If this works, it works. But it's something I'd like to revisit at some point, not happy about all of our relative paths and such, especially since things like running Python with -m can affect it

Yeah, this does rely on running the engine from a specific directory.

Alfred-Mountfield · 2022-01-06T17:31:26Z

+        #       changing `sys.path` here (vs some more complicated way
+        #       of importing files from parent directories of the
+        #       package file's directory).
+        code = ("import pathlib\n" +


This is fixed in a later PR so it's okay

Alfred-Mountfield · 2022-01-06T17:34:26Z

        # that the runner exited.

-        error = "Runner error: " + str(traceback.format_exception(type(exc), exc, tb))
+        error = "Runner error: " + str(traceback.format_exception(*exc_info))


Can we either pass the destructured args, or can we just type hint?

Or at least a comment saying it comes from the function so you can destructure

lgtm-com · 2022-01-06T19:50:29Z

This pull request introduces 1 alert when merging bc657bf into b8274df - view on LGTM.com

new alerts:

1 for Unused import

* Fix flatbuffers conversion during Python init (#112) * Fix package loading in Python runner (#113) * Fix line numbers in Python tracebacks (#114) * Fix behavior execution package Python component init (#115) * Fix messages package Python experiment init (#116) * Fix Python runner init (#117) * Fix Python execution (#214) * Log received inbound messages in Python process * Fix Python state snapshot sync * Display flatc errors from genfbs.py * Fix conflicting TaskMsg flatbuffers definitions * Propagate JS inbound message receive errors * Fix Python state interim sync * Fix Python task execution * Make Python error handling more consistent; update docstrings * Enabling Python tests * Adding Python setup to CI * Building before setting up Python script in CI * Only running Python setup from the root and building all * Running python setup from the package root * Changing python setup run cmd * Specifying server in cargo build in workflow * Running tests from the right cwd * Debugging the contents of the github workspace * Fix Python `context.step()` * Changing output format on integration tests * Fix a variety of Errors around the Python runner (#254) * Fixed init tests * Made behavior index private, and fixed incrementing with multi-language, and fixed access in Python * Fixing behavior index field access in python to use full loading * Fix receiving Python errors in Rust * Removing outdated `server` reference from Rust workflow * Fixing optional GroupIndex on TaskMsg * Programmatically search for libhash_engine_lib.so * Run state interim sync before `run_task` in Python * Disable native python logging * Fix outbound group index deserialization * Fix Python integration tests implementation * Properly check for metaversion in python * Fix neighbor access in Python * Disable behavior reusing * Support index notation for Python * Implement state sync for Python Co-authored-by: litvand <8569704+litvand@users.noreply.github.com> Co-authored-by: Alfred Mountfield <am@hash.ai> Co-authored-by: Tim Diekmann <tim.diekmann@3dvision.de>

Alfred-Mountfield suggested changes Dec 20, 2021

View reviewed changes

Alfred-Mountfield added enhancement labels Dec 20, 2021

litvand changed the base branch from main to dev/python-fixes January 6, 2022 14:57

github-actions bot added the area/content > hash.ai Affects the `hash.ai` informational site (content) label Jan 6, 2022

github-actions bot removed the area/content > hash.ai Affects the `hash.ai` informational site (content) label Jan 6, 2022

Alfred-Mountfield requested review from Alfred-Mountfield and TimDiekmann January 6, 2022 15:17

Fix loading packages in Python runner

b378392

litvand force-pushed the al/py-pkg-init branch from 3a8b191 to b378392 Compare January 6, 2022 15:25

Alfred-Mountfield approved these changes Jan 6, 2022

View reviewed changes

litvand added 2 commits January 6, 2022 21:38

Better variable name

cc4d93f

Comment about function argument

bc657bf

litvand merged commit 84bbc1b into dev/python-fixes Jan 6, 2022

litvand deleted the al/py-pkg-init branch January 6, 2022 19:52

litvand mentioned this pull request Jan 26, 2022

Fix Python runner #229

Merged

vilkinsons added category/enhancement New feature or request and removed C-enhancement labels Jul 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix package loading in Python runner#113

Fix package loading in Python runner#113
litvand merged 3 commits intodev/python-fixesfrom
al/py-pkg-init

litvand commented Dec 20, 2021 •

edited by Alfred-Mountfield

Loading

Uh oh!

lgtm-com bot commented Dec 20, 2021

Uh oh!

Alfred-Mountfield left a comment

Uh oh!

CLAassistant commented Jan 6, 2022 •

edited

Loading

Uh oh!

lgtm-com bot commented Jan 6, 2022

Uh oh!

lgtm-com bot commented Jan 6, 2022

Uh oh!

lgtm-com bot commented Jan 6, 2022

Uh oh!

Alfred-Mountfield Jan 6, 2022

Uh oh!

Alfred-Mountfield Jan 6, 2022

Uh oh!

litvand Jan 6, 2022

Uh oh!

Alfred-Mountfield Jan 6, 2022

Uh oh!

Alfred-Mountfield Jan 6, 2022

Uh oh!

lgtm-com bot commented Jan 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants



		# TODO(permanently?): Keep in sync with PACKAGE_TYPE
		def str_from_pkg_type(t):

Conversation

litvand commented Dec 20, 2021 • edited by Alfred-Mountfield Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🌟 What is the purpose of this PR?

🔗 Related links

🔍 What does this change?

🛡 Tests

❓ How to test this?

Uh oh!

lgtm-com bot commented Dec 20, 2021

Uh oh!

Alfred-Mountfield left a comment

Choose a reason for hiding this comment

Uh oh!

CLAassistant commented Jan 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgtm-com bot commented Jan 6, 2022

Uh oh!

lgtm-com bot commented Jan 6, 2022

Uh oh!

lgtm-com bot commented Jan 6, 2022

Uh oh!

Alfred-Mountfield Jan 6, 2022

Choose a reason for hiding this comment

Uh oh!

Alfred-Mountfield Jan 6, 2022

Choose a reason for hiding this comment

Uh oh!

litvand Jan 6, 2022

Choose a reason for hiding this comment

Uh oh!

Alfred-Mountfield Jan 6, 2022

Choose a reason for hiding this comment

Uh oh!

Alfred-Mountfield Jan 6, 2022

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented Jan 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

litvand commented Dec 20, 2021 •

edited by Alfred-Mountfield

Loading

CLAassistant commented Jan 6, 2022 •

edited

Loading