Skip to content

Conversation

ahmadtariq1
Copy link

Changes in This PR
In sugar-toolkit-gtk3’s activityfactory.py, I’ve implemented:

  • Handling FileNotFoundError: Added a try-except block around the subprocess.Popen call to catch FileNotFoundError.
  • Detection of "sugar-activity": When the exception occurs and involves "sugar-activity,", error is logged to the activity log.
  • Communication of launch failure.

Remaining Work
The following requirements are addressed in a separate PR in Sugar:

Reducing the launcher animation timeout to ~10 seconds.
Displaying error message in launcher.

Testing
Tested on Fedora 41 with Sugar 0.121.
Used calculate-activity(version 44) to verify:
The FileNotFoundError is caught and logged.

Notes
Please review the companion PR in the Sugar repo for the complete view of the fix.

@chimosky kindly review this.

Copy link
Member

@chimosky chimosky left a comment

Choose a reason for hiding this comment

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

Reviewed, not tested.

reply_handler=self._no_reply_handler,
error_handler=self._notify_launch_error_handler)

Copy link
Member

Choose a reason for hiding this comment

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

An unnecessary change.

shell.NotifyLaunchFailure(activity_id,
reply_handler=reply_handler_cb,
error_handler=error_handler_cb)
error_handler=error_handler_cb)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the No new line at EOF, you can do so with echo >> activityfactory.py.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also see if you can figure out how it happened; we don't want unnecessary changes, it consumes reviewing time, and makes git bisect and git blame slightly more interesting than we can emotionally deal with.

@quozl
Copy link
Contributor

quozl commented Apr 8, 2025

  • commit message is almost empty, please rewrite it and force push; include problem being solved (not links), and how you solve it. See Making Commits.
  • using D-Bus to send back the launch failure seems a bit odd, please explain why you chose that,
  • the test for sugar-activity in the error is frankly strange, and will also trigger if sugar-activity3 is in the error,
  • why not check for sugar-activity in exec?

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.

3 participants