Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/pyinstaller #19

Merged
merged 33 commits into from
Feb 12, 2024
Merged

Feat/pyinstaller #19

merged 33 commits into from
Feb 12, 2024

Conversation

zzjc1234
Copy link
Collaborator

@zzjc1234 zzjc1234 commented Sep 2, 2023

This pr modified canvas_app s.t. the code can be packed with pyinstaller

@zzjc1234 zzjc1234 requested a review from linsyking September 2, 2023 07:44
@linsyking
Copy link
Owner

Have you tested whether pyinstaller can work on this?

@zzjc1234
Copy link
Collaborator Author

zzjc1234 commented Sep 2, 2023

Yes, the exe works well. I disablesd the output in console, I think users may not need it.

@zzjc1234
Copy link
Collaborator Author

zzjc1234 commented Sep 2, 2023

The updator may have some unknown problem, but the updator.py should be modified anyway. Maybe I will fix it in another pr.

@linsyking
Copy link
Owner

For executables the old updater doesn't work because it is based on git

@linsyking
Copy link
Owner

Maybe we can just remove git update logic when building for executables.

@zzjc1234
Copy link
Collaborator Author

zzjc1234 commented Sep 2, 2023

It seems that I didn't solve the dependency well, currently the exe only works under the dir with canvas_mgr.py and etc. I will try to fix it.

@linsyking
Copy link
Owner

Also we need to migrate some arguments from uvicorn to the executables, eg. the port and base_url

@linsyking
Copy link
Owner

linsyking commented Sep 2, 2023

I think we can remove the updater support. It is not very helpful and needs git dependency.

@linsyking
Copy link
Owner

do not merge until everything is set

@linsyking
Copy link
Owner

Please fix the build error.

@linsyking
Copy link
Owner

I think we should first merge it to the dev branch otherwise current users will be affected

@linsyking linsyking changed the base branch from main to dev September 7, 2023 14:36
start.py Show resolved Hide resolved
start.py Outdated
if usr_host == "":
usr_host = "localhost"
if usr_port == "":
usr_port = 9283
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not write duplicate code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@linsyking linsyking self-requested a review September 7, 2023 14:52
@linsyking linsyking self-assigned this Sep 7, 2023
@linsyking linsyking added the enhancement New feature or request label Sep 7, 2023
@zzjc1234 zzjc1234 assigned linsyking and unassigned linsyking Jan 16, 2024
@zzjc1234 zzjc1234 merged commit a4a3306 into dev Feb 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants