-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add a PyInstaller build for unsupported distros #27
Conversation
brainframe/cli/frozen_utils.py
Outdated
if is_frozen(): | ||
path = _pyinstaller_tmp_path() / target | ||
if not path.is_file(): | ||
raise FileNotFoundError() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should give this exceptions more info:
import errno
import os
raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), filename)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually I'm down to provide a lot of information in exceptions, but for this use-case it would all go unused always. The caller always knows what file it's asking for and would never provide the message to the user directly because we want a translated, user-readable error. If you gotta use errno
to use FileNotFoundError
properly, maybe I should go back to a custom exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My vote is to add the errno
information to the FileNotFoundError
, but it's not a deal-breaker if it doesn't happen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just make sure to check those boxes before merging!
Manual tests looking good! |
This adds configuration to build the CLI as a PyInstaller executable along with the necessary supporting code. The PyInstaller executable would be an option for users of distributions whose package manager we don't have support for.
Since there's no automatic update mechanism available, this also implements a new
self-update
command that pulls the latest version of the CLI and replaces the current version with that one.Unfortunately, this version of the CLI does not ship with its own version of Docker Compose. Packaging Compose into PyInstaller isn't straightforward, and it can't be run as a module because the Python interpreter is embedded in the binary. Instead, the expectation is that the user installs Compose with whatever mechanism they prefer.
See #13, which this PR does not fix on its own.
To-do Before Merge
self-update
command to pull from a real URL