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

feature(installer): update installer version and improve desktop version launcher #2157

Merged
merged 45 commits into from
Oct 11, 2024

Conversation

sylvlecl
Copy link
Member

@sylvlecl sylvlecl commented Sep 25, 2024

  • updates installer submodule to benefit from last fixes and v2.18 handling
  • make application exit when the server process ends, instead of hanging
  • detect when server is already running
  • separate one popup for "starting" and another for "started"
  • fix alembic handling in pyinstaller:
    alembic must not be defined as a script, otherwise it's executed when the application
    quits. Instead, it must just be analyzed separately to get its dependencies

Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Otherwise, alembic is actually launched after the gui.py script ends.
We still need to analyze dependencies of all alembic scripts though.

Also, make the application exit when the server process ends.

Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 30, 2024
@sylvlecl sylvlecl changed the title feature(installer): update installer version feature(installer): update installer version and improve desktop version launcher Oct 1, 2024
@sylvlecl sylvlecl marked this pull request as ready for review October 1, 2024 11:36
@sylvlecl sylvlecl requested a review from MartinBelthle October 1, 2024 11:36
Signed-off-by: Sylvain Leclerc <[email protected]>
MartinBelthle
MartinBelthle previously approved these changes Oct 2, 2024
Copy link
Contributor

@MartinBelthle MartinBelthle left a comment

Choose a reason for hiding this comment

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

The gui.py file is really hard to understand IMO.
I wouldn't mind more doc cause I'm not familiar with this code 😃

@@ -4,6 +4,7 @@ on:
branches:
- "master"
- "hotfix/**"
- "feature/update-installer"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be removed or we could add more generic branch name such as installer/** or deploy/**

migration_files = [str(f) for f in migrations_dir.iterdir() if f.is_file() and f.suffix == '.py']
# We need to analyze all alembic files to be sure the migration phase works fine:
# alembic loads version files by their path, so we need to add them as "data" to the package,
# but all the dependencies they use need to be included also, wo we need to perform a
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: so we need

binaries=binaries,
datas=[('./resources', './resources'), ('./alembic', './alembic')],
binaries=[],
datas=[('./resources', './resources'), ('./alembic', './alembic'), ('./alembic.ini', './')],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you add ./ ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Before we had this: binaries = [('./alembic.ini', './alembic.ini')] which created a folder called "alembic.ini" in which the file was placed.

Now the file is correctly placed at the root

migration_files = [str(f) for f in migrations_dir.iterdir() if f.is_file() and f.suffix == '.py']
# We need to analyze all alembic files to be sure the migration phase works fine:
# alembic loads version files by their path, so we need to add them as "data" to the package,
# but all the dependencies they use need to be included also, wo we need to perform a
Copy link
Contributor

Choose a reason for hiding this comment

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

same typo

antarest/gui.py Outdated
quit_action.triggered.connect(app.quit)

# Adding options to the System Tray
def handle_action(reason: int) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

This function/design is really weird but I'm far from understanding this file

@pull-request-size pull-request-size bot added size/XL and removed size/L labels Oct 2, 2024
@sylvlecl sylvlecl marked this pull request as draft October 2, 2024 19:56
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
@sylvlecl sylvlecl marked this pull request as ready for review October 11, 2024 15:20
Signed-off-by: Sylvain Leclerc <[email protected]>
Signed-off-by: Sylvain Leclerc <[email protected]>
@sylvlecl sylvlecl merged commit f41b669 into dev Oct 11, 2024
10 of 11 checks passed
@sylvlecl sylvlecl deleted the feature/update-installer branch October 11, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants