Skip to content

Store process information and manage application logging#94

Merged
antirotor merged 54 commits intodevelopfrom
enhancement/store-process-info
Oct 1, 2025
Merged

Store process information and manage application logging#94
antirotor merged 54 commits intodevelopfrom
enhancement/store-process-info

Conversation

@antirotor
Copy link
Copy Markdown
Member

@antirotor antirotor commented Aug 7, 2025

Changelog Description

Store launched process information in a local database and add process monitoring/output manager.

Additional review information

When process is run, this will store information about it in local sqlite3 database that can be later on used by other parts of AYON. It also passes temporary file as stdout and stderr handle, storing its path along the process information.

All this information can be used to read out what the launched process is outputting in situations where it doesn't open its own console. For this Process Monitoring tool is added:

image

This tool can be run using Process Monitor item in Tray menu.

What it does:

  • periodically refresh process list from database
  • checks if process is still running
  • can open file with process output and reload it periodically
  • you can clean output files for any inactive process
  • it colorize output if it contains ANSI sequences
  • it can handle wrapping shell scripts as executables (scripts that will at the end launch the "real" executable)

Further enhancements:

Shell scripts and passing of PID back to AYON

When launching applications through shell scripts on Linux, the process ID returned by the app_launcher (mid-process) corresponds to the shell process, not the actual application process that the shell script launches.

Architecture Overview

  • AYON Applications Manager (manager.py) creates launch contexts and manages application processes
  • App Launcher (app_launcher.py/app_launcher.cpp in ayon-launcher) is the mid-process that handles detached process launching on Linux
  • Shell Scripts are often used as executables that set up environment and launch actual applications

Solution

The solution implements a PID file convention that allows shell scripts to communicate the actual application PID back to AYON through the app_launcher mid-process.

How it Works

  1. PID File Creation: The AYON manager creates a temporary PID file and provides its path to the app_launcher via JSON data
  2. Environment Variable: The app_launcher passes the PID file path to shell scripts via AYON_PID_FILE environment variable
  3. Shell Script Responsibility: Shell scripts can write the actual application PID to this file after launching the application
  4. PID Discovery: The app_launcher waits briefly and checks for an updated PID in the file, using it if found

Integration Requirements

For shell scripts to take advantage of this feature:

  1. Check if $AYON_PID_FILE environment variable exists
  2. Launch your application in the background using &
  3. Capture the PID using $! (bash) or equivalent
  4. Write the PID to the file specified by $AYON_PID_FILE

Bash Example

#!/bin/bash

# Launch the actual application in background
/path/to/actual/application "$@" &

# Get the PID of the actual application
APP_PID=$!

# Write the PID to the AYON PID file if available
if [ -n "$AYON_PID_FILE" ]; then
    echo "$APP_PID" > "$AYON_PID_FILE"
fi

# Wait for the application to finish (optional)
wait $APP_PID

Warning

For retrieving process status, it is using psutil tool that you need in your dependencies. That means building new dependency package. You can add psutil manually but keep in mind that it is binary dependency and thus it will work only with compatible Python host (if you build it for python 3.9 it is unlikely that it will also work in 3.11).

Testing notes:

You should be able to run any DCC as usual and see its output in Process Manager. It works on Linux and macos. Try to wrap and execute some application in shell script as shown in the example above - Process Manager should still pick the correct process id and monitor the process. Removing of selected item/All inactive items should work too. If you use ANSI sequences to colorize your shell script, you should see that also in the output (it supports even bold, underline, ... styles).

@antirotor antirotor self-assigned this Aug 7, 2025
@antirotor antirotor added the type: enhancement Improvement of existing functionality or minor addition label Aug 7, 2025
@antirotor
Copy link
Copy Markdown
Member Author

I've not set it as Draft even though it has some issues on other platforms. It works in usable state on Windows and it is good to have some testing feedback while working on other platforms issues.

@antirotor antirotor requested review from Copilot and iLLiCiTiT August 7, 2025 17:25

This comment was marked as outdated.

@antirotor antirotor requested a review from Copilot August 8, 2025 17:22

This comment was marked as outdated.

antirotor and others added 5 commits August 8, 2025 19:24
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds process information storage and management capabilities to AYON applications. It introduces a local SQLite database to track launched processes and provides a Process Monitor UI for viewing and managing these processes.

Key changes:

  • Store launched process information in a local SQLite database with process details like PID, output file paths, and status
  • Add a Process Monitor GUI tool accessible from the tray menu for viewing process status and output logs
  • Implement process status checking using psutil with fallbacks to system tools

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
client/pyproject.toml Adds psutil dependency for process status checking
client/ayon_applications/ui/process_monitor.py New Process Monitor GUI with table view, output display, and cleanup functionality
client/ayon_applications/manager.py Adds ProcessInfo dataclass and database operations for storing/retrieving process information
client/ayon_applications/addon.py Integrates Process Monitor as a tray action in the AYON interface



@dataclass
class ProcessInfo:
Copy link
Copy Markdown
Member

@iLLiCiTiT iLLiCiTiT Sep 17, 2025

Choose a reason for hiding this comment

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

Should have id or process_id with unique value. There is no column with a unique value in the database at this moment...

I would expect you can see output of already closed applications, in that case the hash don't have to work, and calculation of the hash is unnecessary slowdown, compared to explicit simple id (especially if this is considered to be on server -> I guess that based on the site id).

Copy link
Copy Markdown
Member

@iLLiCiTiT iLLiCiTiT Sep 17, 2025

Choose a reason for hiding this comment

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

It would also simplified finding the process object to avoid using hash of values that don't have to be unique (even if are combined from multiple columns).

Copy link
Copy Markdown
Member Author

@antirotor antirotor Sep 17, 2025

Choose a reason for hiding this comment

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

there is a hash as primary key. It is sha256 of process name, pid, executable and start time.

Copy link
Copy Markdown
Member

@iLLiCiTiT iLLiCiTiT Sep 17, 2025

Choose a reason for hiding this comment

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

And I don't understand why that was used over using uuid? The hash is potential danger from my perspective (especially when it won't be used only for local site database file). You also have to calculate it all the time you want to get information about the process.

cwd: str
pid: Optional[int] = None
active: bool = False
output: Optional[Path] = None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this the content of stdout/stderr files? Shouldn't that be loaded in a separate way (so you don't have to open the large files to list processes).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is not a content, it is a path. And it is loaded in a separate way in the Manger tool.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then the attribute name is not clear and confusing, should be output_path.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

output is typed as Path. Adding it to variable name is IMO redundant in most modern IDEs. In the same line it should be executable_path or start_timestamp_utc. I would agree if it was plain str

@iLLiCiTiT
Copy link
Copy Markdown
Member

iLLiCiTiT commented Sep 22, 2025

Why were all my comments resolved?

I don't agree the way the processes are stored I'm strongly against using hash, that is not safe and requires to be calculated using other values from table, instead of being explicitly unique, we should replace that with "real" id using uuid or different id system.

I don't understand why we do store site id to the data, there is no reason to because the database file is stored to site and user specific location, if that's because it will be stored to server "some time" then using hash is even more dangerous and should be removed. Also if we do plan to do that change in future, then we should do that in future when we do it on server. Right now we're adding data thay might be stored in a different way on server. We should be prepared for it, but we should not pre-implement it if we don't know how that will work.

@antirotor
Copy link
Copy Markdown
Member Author

Why were all my comments resolved?

I forgot to push the changes

I don't agree the way the processes are stored I'm strongly against using hash, that is not safe and requires to be calculated using other values from table, instead of being explicitly unique, we should replace that with "real" id using uuid or different id system.

Hash is unique enough. I don't see how you could achieve conflicting hashes when you don't create milions of processes within second.

I don't understand why we do store site id to the data, there is no reason to because the database file is stored to site and user specific location, if that's because it will be stored to server "some time" then using hash is even more dangerous and should be removed. Also if we do plan to do that change in future, then we should do that in future when we do it on server. Right now we're adding data thay might be stored in a different way on server. We should be prepared for it, but we should not pre-implement it if we don't know how that will work.

That was removed.

@antirotor antirotor merged commit c783419 into develop Oct 1, 2025
1 check passed
@antirotor antirotor deleted the enhancement/store-process-info branch October 3, 2025 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement Improvement of existing functionality or minor addition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants