Sets up the hamilton ui to run on sqlite + be installed/run with a pip install command #942

elijahbenizzy · 2024-06-08T00:28:36Z

You can run it with:

python manage.py runserver --settings=server.settings_mini 0.0.0.0:8241 from ui/backend/server
npm run start from ui/frontend/server

Changes

How I tested this

Notes

Checklist

PR has an informative and human-readable title (this will be pulled into the release notes)
Changes are limited to a single goal (no scope creep)
Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
Any change in functionality is tested
New functions are documented (with a description, list of inputs, and expected output)
Placeholder code is flagged / future TODOs are captured in comments
Project documentation has been updated if adding/changing functionality.

elijahbenizzy · 2024-06-10T16:17:31Z

This is working -- you can:

pip install "sf-hamilton[serve]==1.65.0rc0"
hamilton-serve

We: 1. get it to work with sqllite 2. Package up UI 3. Add a hamilton build script This way someone can just run pip install hamilton[ui] && hamilton ui and it'll work. Design decisions: 1. Switch fields between postgres and sqllite -- this is a bit of a hack but it's the only way to truly dynamically swap between models. We use a custom array field in sqllite and use standard array field in postgres. 2. Use a deploy/build script -- this is the only way to get it to work Still need to add to docs + improve deployment/installation mechanism

elijahbenizzy · 2024-06-10T22:04:25Z

TODO:

Docs -- add to the UI section
README -- add getting started to the front page
Test out the docker iamges so they still world
Add in example projects (skipping out for now due to timing)

ellipsis-dev

👍 Looks good to me! Reviewed everything up to de201bb in 48 seconds

More details

Looked at 1892 lines of code in 33 files
Skipped 0 files when reviewing.
Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_9UR0UCedarVhF4fN

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

MANIFEST.in

setup.py

ellipsis-dev

👍 Looks good to me! Incremental review on 025945c in 48 seconds

More details

Looked at 19 lines of code in 1 files
Skipped 1 files when reviewing.
Skipped posting 1 drafted comments based on config settings.

1. setup.py:53

Draft comment:
The removal of package_data in the setup.py file might affect the packaging of the UI or other components. Please ensure that this change does not unintentionally remove necessary files from the package.
Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_LSqGkXF1WxehFErL

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev

👍 Looks good to me! Incremental review on 1e84407 in 39 seconds

More details

Looked at 360 lines of code in 12 files
Skipped 0 files when reviewing.
Skipped posting 1 drafted comments based on config settings.

1. ui/backend/server/commands.py:60

Draft comment:
The run function in commands.py has been modified to include a new parameter no_open and logic to open the browser when the server is ready. This is a significant change in behavior. Please ensure this is documented in the PR description and tested thoroughly.
Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_ZMjkzWZx0EpB0UKI

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ui/backend/setup.py

skrawcz

Just need to clean up the UI setup.py I think.

ellipsis-dev

👍 Looks good to me! Incremental review on af7789d in 52 seconds

More details

Looked at 395 lines of code in 12 files
Skipped 0 files when reviewing.
Skipped posting 1 drafted comments based on config settings.

1. ui/backend/server/commands.py:62

Draft comment:
The environment variable DJANGO_SETTINGS_MODULE is set to hamilton.server.server.settings_mini. Ensure the path is correct as it seems to include 'server' twice which might be incorrect.
Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_3fDK1ql4mQZWbYwB

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev

❌ Changes requested. Incremental review on 1ffaabd in 1 minute and 7 seconds

More details

Looked at 317 lines of code in 12 files
Skipped 0 files when reviewing.
Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_ShzoXckHanzEdMLB

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev · 2024-06-13T16:14:27Z

ui/backend/server/commands.py

+
+def run(port: int, base_dir: str, no_migration: bool, no_open: bool):
+    env = {
+        "DJANGO_SETTINGS_MODULE": "hamilton.server.server.settings_mini",


The Django settings module path seems incorrect. It points to 'hamilton.server.server.settings_mini', which likely does not exist due to the double 'server' directory. Please correct the path to ensure the Django application can start correctly.

Suggested change

"DJANGO_SETTINGS_MODULE": "hamilton.server.server.settings_mini",

"DJANGO_SETTINGS_MODULE": "hamilton_ui.server.settings_mini",

This is much cleaner/decoupled from the hamilton core capabilities. See README for installation/publishing instructions.

ellipsis-dev

👍 Looks good to me! Incremental review on 9f0a7cd in 1 minute and 29 seconds

More details

Looked at 317 lines of code in 12 files
Skipped 0 files when reviewing.
Skipped posting 1 drafted comments based on config settings.

1. ui/README.md:90

Draft comment:
The PR title and description should be updated to accurately reflect all the significant changes made, not just the setup to run on SQLite.
Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_bgb9fHkYruO1TP8g

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

elijahbenizzy force-pushed the simplify-ui-local-dev branch 9 times, most recently from a504907 to 02b058a Compare June 10, 2024 21:49

elijahbenizzy force-pushed the simplify-ui-local-dev branch from 02b058a to 0a26002 Compare June 10, 2024 21:58

Updates readme to highlight ease of running hamilton UI

de201bb

elijahbenizzy force-pushed the simplify-ui-local-dev branch from 405ff8e to de201bb Compare June 11, 2024 03:10

elijahbenizzy requested a review from skrawcz June 11, 2024 03:12

elijahbenizzy marked this pull request as ready for review June 11, 2024 03:29

ellipsis-dev bot reviewed Jun 11, 2024

View reviewed changes

skrawcz reviewed Jun 11, 2024

View reviewed changes

MANIFEST.in Outdated Show resolved Hide resolved

skrawcz reviewed Jun 11, 2024

View reviewed changes

setup.py Outdated Show resolved Hide resolved

elijahbenizzy changed the title ~~Sets up the hamilton ui to run on sqlite (WIP)~~ Sets up the hamilton ui to run on sqlite Jun 11, 2024

Cleans up packages + commented out code

025945c

ellipsis-dev bot reviewed Jun 11, 2024

View reviewed changes

ellipsis-dev bot reviewed Jun 13, 2024

View reviewed changes

skrawcz reviewed Jun 13, 2024

View reviewed changes

ui/backend/setup.py Outdated Show resolved Hide resolved

skrawcz reviewed Jun 13, 2024

View reviewed changes

ui/backend/setup.py Outdated Show resolved Hide resolved

skrawcz approved these changes Jun 13, 2024

View reviewed changes

elijahbenizzy force-pushed the simplify-ui-local-dev branch from 1e84407 to af7789d Compare June 13, 2024 16:07

ellipsis-dev bot reviewed Jun 13, 2024

View reviewed changes

elijahbenizzy force-pushed the simplify-ui-local-dev branch from af7789d to 1ffaabd Compare June 13, 2024 16:13

ellipsis-dev bot reviewed Jun 13, 2024

View reviewed changes

Reorganizes hamilton ui to be in its own package

9f0a7cd

This is much cleaner/decoupled from the hamilton core capabilities. See README for installation/publishing instructions.

elijahbenizzy force-pushed the simplify-ui-local-dev branch from 1ffaabd to 9f0a7cd Compare June 13, 2024 16:18

ellipsis-dev bot reviewed Jun 13, 2024

View reviewed changes

elijahbenizzy changed the title ~~Sets up the hamilton ui to run on sqlite~~ Sets up the hamilton ui to run on sqlite + be installed/run with a pip install command Jun 13, 2024

elijahbenizzy merged commit 3f96279 into main Jun 13, 2024
26 checks passed

elijahbenizzy deleted the simplify-ui-local-dev branch June 13, 2024 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sets up the hamilton ui to run on sqlite + be installed/run with a pip install command #942

Sets up the hamilton ui to run on sqlite + be installed/run with a pip install command #942

elijahbenizzy commented Jun 8, 2024 •

edited

Loading

elijahbenizzy commented Jun 10, 2024

elijahbenizzy commented Jun 10, 2024 •

edited

Loading

ellipsis-dev bot left a comment

ellipsis-dev bot left a comment

ellipsis-dev bot left a comment

skrawcz left a comment

ellipsis-dev bot left a comment

ellipsis-dev bot left a comment

ellipsis-dev bot Jun 13, 2024

ellipsis-dev bot left a comment

	"DJANGO_SETTINGS_MODULE": "hamilton.server.server.settings_mini",
	"DJANGO_SETTINGS_MODULE": "hamilton_ui.server.settings_mini",

Sets up the hamilton ui to run on sqlite + be installed/run with a pip install command #942

Sets up the hamilton ui to run on sqlite + be installed/run with a pip install command #942

Conversation

elijahbenizzy commented Jun 8, 2024 • edited Loading

Changes

How I tested this

Notes

Checklist

elijahbenizzy commented Jun 10, 2024

elijahbenizzy commented Jun 10, 2024 • edited Loading

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

skrawcz left a comment

Choose a reason for hiding this comment

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot Jun 13, 2024

Choose a reason for hiding this comment

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

elijahbenizzy commented Jun 8, 2024 •

edited

Loading

elijahbenizzy commented Jun 10, 2024 •

edited

Loading