Skip to content

Conversation

@terencehonles
Copy link
Contributor

This change updates the script initialization to not fail on import errors, to finish without checking if --no-gui or --external-auth has been passed in the command-line, and instead raises an error when the application is initializing if running in GUI mode and the requirements are not met.

Minor changes also included:

  • The --no-gui option is now stored as gui and defaults to True which changes comparisons to not be double negatives but retains the existing behavior.
  • The App instance is allowed a single argument which is the command-line to parse, which allows starting the app with arguments from an import.

@terencehonles
Copy link
Contributor Author

This will also allow packaging without all the requirements and this check https://github.com/simonrob/email-oauth2-proxy/pull/199/files#diff-8068eda2feddbc61595ced559aa73f8502813cf5cb1190a21e111ab1e8d19e88R71

This change updates the script initialization to not fail on import
errors, to finish without checking if `--no-gui` or `--external-auth`
has been passed in the command-line, and instead raises an error when
the application is initializing if running in GUI mode and the
requirements are not met.

Minor changes also included:

- The `--no-gui` option is now stored as `gui` and defaults to `True`
  which changes comparisons to not be double negatives but retains the
  existing behavior.

- The App instance is allowed a single argument which is the
  command-line to parse, which allows starting the app with arguments
  from an import.
@terencehonles terencehonles force-pushed the allow-imports-without-gui-requirements branch from 8cc4b31 to 6e461e3 Compare October 16, 2023 08:39
Copy link
Owner

@simonrob simonrob left a comment

Choose a reason for hiding this comment

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

One benefit of the previous approach was that failed imports were more visible because they printed an error. With this new approach it's a lot harder to see which import is missing (and with #199 merged there won't necessarily be an easily-available requirements.txt to fix this).

At the moment, apart from for pystray and AppKit, is there any need to separate all of the imports and catch errors independently? If any single one fails and --no-gui isn't specified then we're going to exit anyway. So, perhaps a better approach is to bring back the original ArgumentParser, but use it to decide whether to display an error or not (i.e., if in GUI mode and imports fail).

@simonrob simonrob merged commit 498ac99 into simonrob:main Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants