Skip to content

Add /topic/ prefix to ActiveMQ destinations, xrootd optional dependencies, etc. - #2

Open
tntakahashi wants to merge 8 commits into
BNLNPPS:mainfrom
tntakahashi:main
Open

Add /topic/ prefix to ActiveMQ destinations, xrootd optional dependencies, etc.#2
tntakahashi wants to merge 8 commits into
BNLNPPS:mainfrom
tntakahashi:main

Conversation

@tntakahashi

Copy link
Copy Markdown
Contributor
  • Add /topic/ prefix to ActiveMQ destinations
  • Make testbed-config path and watch directory path configurable via args or env vars
  • Guard setup_environment to avoid double loading .env in swf_fastmon_agent
  • Add xrootd to optional dependencies in pyproject.toml for remote file access support
  • Update fastmon-client to place TF files downloaded via XRootD into the directory specified by --xrootd-target-dir or ENV: FASTMON_CLIENT_XROOTD_TARGET_DIR
  • Modify fastmon-agent to creat TF files at the location specified by --tf-base-url or ENV:FASTMON_TF_BASE_URL

Make testbed-config path and watch directory path configurable via args or env vars
Guard setup_environment to avoid double loading .env in swf_fastmon_agent
Add xrootd to optional dependencies in pyproject.toml for remote file access support
Update fastmon-client to place TF files downloaded via XRootD into the directory specified by --xrootd-target-dir or ENV: FASTMON_CLIENT_XROOTD_TARGET_DIR
Modify fastmon-agent to creat TF files at the location specified by --tf-base-url or ENV:FASTMON_TF_BASE_URL

Copilot AI left a comment

Copy link
Copy Markdown

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 updates the fast monitoring agent/client to support ActiveMQ /topic/ destinations, configurable runtime paths via CLI/env vars, and optional XRootD-based remote TF file handling.

Changes:

  • Add /topic/ prefix defaults for the fastmon agent’s ActiveMQ subscription and publish destinations.
  • Add CLI/env configuration for testbed config path, watch directories, and TF base URL in the agent.
  • Add optional XRootD dependency and implement XRootD-based TF download (client) / TF creation (agent utils).

Reviewed changes

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

File Description
src/swf_fastmon_client/main.py Adds optional XRootD integration and configurable TF download target directory.
src/swf_fastmon_agent/main.py Updates ActiveMQ destination defaults and adds CLI/env configuration for config/watch/TF base URL; guards .env loading.
src/swf_fastmon_agent/fastmon_utils.py Extends TF simulation to incorporate tf_base_url and optionally create TF files via XRootD.
pyproject.toml Adds xrootd as an optional dependency extra.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/swf_fastmon_agent/fastmon_utils.py Outdated
Comment thread src/swf_fastmon_client/main.py Outdated
Comment on lines +29 to +35
def expand_all(s: str, max_iter: int = 10) -> str:
for _ in range(max_iter):
new = os.path.expandvars(s)
if new == s:
return new
s = new
return s

Copilot AI Mar 11, 2026

Copy link

Choose a reason for hiding this comment

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

expand_all is typed as str but is called with os.getenv(...), which can return None. As written, os.path.expandvars(None) will raise a TypeError and prevent the client from starting when the env var is unset. Consider accepting Optional[str] and returning None/"" when the input is None (or guard at call sites).

Copilot uses AI. Check for mistakes.
Comment thread src/swf_fastmon_client/main.py Outdated
Comment thread src/swf_fastmon_client/main.py Outdated
Comment thread src/swf_fastmon_client/main.py
Comment thread src/swf_fastmon_agent/fastmon_utils.py Outdated
Comment on lines +320 to +336
if HAS_XROOTD:
# create empty TF file
try:
parsed = urlparse(tf_filename)
fs = client.FileSystem(f'{parsed.scheme}://{parsed.netloc}')
f = client.File()

# To avoid a "permission denied" error, perform the file operation locally by using the path (= parsed.path) without the scheme and netloc.
status, _ = f.open(
parsed.path, # tf_filename
client.flags.OpenFlags.NEW
| client.flags.OpenFlags.WRITE
| client.flags.OpenFlags.MAKEPATH)
if not status.ok:
raise RuntimeError(f"Failed to create TF file = {tf_filename}, status = {status.message}")
except Exception as e:
print(f"An exception occurred while creating the TF file: {e}")

Copilot AI Mar 11, 2026

Copy link

Choose a reason for hiding this comment

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

The XRootD file-creation branch is gated only by HAS_XROOTD, not by the URL scheme. If tf_base_url is file://... (including the default in main.py), this will still try to create the file via XRootD and will error for local paths. Consider checking urlparse(tf_filename).scheme (e.g., only use XRootD for root:// URLs) and creating local files with standard filesystem APIs for file:// targets.

Suggested change
if HAS_XROOTD:
# create empty TF file
try:
parsed = urlparse(tf_filename)
fs = client.FileSystem(f'{parsed.scheme}://{parsed.netloc}')
f = client.File()
# To avoid a "permission denied" error, perform the file operation locally by using the path (= parsed.path) without the scheme and netloc.
status, _ = f.open(
parsed.path, # tf_filename
client.flags.OpenFlags.NEW
| client.flags.OpenFlags.WRITE
| client.flags.OpenFlags.MAKEPATH)
if not status.ok:
raise RuntimeError(f"Failed to create TF file = {tf_filename}, status = {status.message}")
except Exception as e:
print(f"An exception occurred while creating the TF file: {e}")
# Create an empty TF file if possible. Use XRootD only for root:// URLs,
# and fall back to local filesystem APIs for file:// or plain paths.
parsed = urlparse(tf_filename)
if parsed.scheme == "root" and HAS_XROOTD:
try:
f = client.File()
# To avoid a "permission denied" error, perform the file operation locally
# by using the path (= parsed.path) without the scheme and netloc.
status, _ = f.open(
parsed.path,
client.flags.OpenFlags.NEW
| client.flags.OpenFlags.WRITE
| client.flags.OpenFlags.MAKEPATH,
)
if not status.ok:
raise RuntimeError(
f"Failed to create TF file = {tf_filename}, status = {status.message}"
)
except Exception as e:
print(f"An exception occurred while creating the TF file via XRootD: {e}")
else:
try:
# Handle local files (file:// URLs or plain paths) via the filesystem.
if parsed.scheme:
local_path = Path(parsed.path)
else:
local_path = Path(tf_filename)
local_path.parent.mkdir(parents=True, exist_ok=True)
local_path.touch(exist_ok=True)
except Exception as e:
print(f"An exception occurred while creating the TF file locally: {e}")

Copilot uses AI. Check for mistakes.
Comment thread src/swf_fastmon_agent/fastmon_utils.py Outdated
Comment thread src/swf_fastmon_agent/fastmon_utils.py
Comment thread src/swf_fastmon_client/main.py Outdated
Comment thread src/swf_fastmon_agent/main.py Outdated

@michmx michmx left a comment

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.

A couple of comments on top of the catches by copilot

Comment thread src/swf_fastmon_client/main.py Outdated
Comment thread src/swf_fastmon_client/main.py
michmx and others added 3 commits April 10, 2026 07:36
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>
Comment thread src/swf_fastmon_agent/main.py Outdated
Comment thread src/swf_fastmon_client/main.py
Comment thread src/swf_fastmon_client/main.py Outdated
@tntakahashi
tntakahashi requested a review from michmx April 17, 2026 14:40
@tntakahashi

Copy link
Copy Markdown
Contributor Author

The swf-monitor currently running on pandaserver02 has the February 27 commit bf27fbc
applied, which changed it to accept STF files by file name rather than by UUID, so it appears that the swf-fastmon-agent code also needs to be updated.

Update FastMon TF metadata and FastMonFile API payloads to use the
parent STF filename when registering TF files with swf-monitor. Keep the
STF UUID in metadata for traceability, but send the filename expected by
the monitor serializer to resolve FastMonFile.stf_file.

Also update TF notification payloads and tests for the
filename-based parent STF lookup.
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