Skip to content

feat: Persist GUI app settings between executions#1169

Merged
bdferris-v2 merged 6 commits intoMobilityData:masterfrom
bdferris-v2:issue/1165/persist
May 27, 2022
Merged

feat: Persist GUI app settings between executions#1169
bdferris-v2 merged 6 commits intoMobilityData:masterfrom
bdferris-v2:issue/1165/persist

Conversation

@bdferris-v2
Copy link
Copy Markdown
Collaborator

Currently, the GUI app does not remember settings (input and output paths, advanced settings) between different executions of the app. This PR adds support for persisting settings between runs of the validator, using the Java Preferences API for storage.

Closes #1165.

Please make sure these boxes are checked before submitting your pull request - thanks!

Copy link
Copy Markdown
Contributor

@maximearmstrong maximearmstrong left a comment

Choose a reason for hiding this comment

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

Everything is fine with the code, but I encounter a strange behavior on macOS. After building the package, I use the .dmg. The application opens then closes automatically, without even displaying the application window. Here are the logs:

May 27, 2022 12:59:38 PM org.mobilitydata.gtfsvalidator.app.gui.Main main
INFOS: gtfs-validator: start
May 27, 2022 12:59:39 PM org.mobilitydata.gtfsvalidator.app.gui.Main main
INFOS: gtfs-validator: exit

Any ideas on what the problem might be?

@bdferris-v2
Copy link
Copy Markdown
Collaborator Author

The issue was that I hadn't added the "java.prefs" module to the list of required modules in the apps module spec. I fixed that in 09d79fb.

But more broadly, the exception that was thrown was getting added to the run.log because it was an uncaught exception in the UI thread. In 101b2da, I added a custom uncaught exception handler that logs these exceptions (and anything else we missed) into the application log. I've verified that the logging properly logs the missing error above. Doesn't stop the crash but does make it easier to debug.

This all makes me wonder if we can actually run the full application in the CI environment somehow to catch issues like this. Not sure if a GUI application would be supported though...

@maximearmstrong
Copy link
Copy Markdown
Contributor

This all makes me wonder if we can actually run the full application in the CI environment somehow to catch issues like this. Not sure if a GUI application would be supported though...

If we can do that, that would be great. We could open an issue to investigate it. I don't think it's required for this PR though.

Copy link
Copy Markdown
Contributor

@maximearmstrong maximearmstrong left a comment

Choose a reason for hiding this comment

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

LGTM

@bdferris-v2 bdferris-v2 merged commit 75c64a9 into MobilityData:master May 27, 2022
@bdferris-v2 bdferris-v2 deleted the issue/1165/persist branch May 27, 2022 18:59
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.

Persist GUI app settings between executions

2 participants