Skip to content

Conversation

@tntakahashi
Copy link
Contributor

This PR adds ActiveMQ connection logic for continuous mode in swf-fastmon-agent,
and fixes two issues in swf-fastmon-client.

Details

  • Added ActiveMQ connection logic when running in continuous mode.
  • Removed default value from monitor_url so that the environment variable is properly used.
  • Added self.logger definition to suppress attribute errors.

  Add ActiveMQ connection in continuous mode
- swf-fastmon-client:
  - Change the default value of monitor_url to None so that environmental variable SWF_MONITOR_URL is properly used
  - Define missing self.logger
Copy link
Member

@michmx michmx left a comment

Choose a reason for hiding this comment

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

Works as it is, just adding a few comments in the implementation.

@app.command()
def start(
monitor_url: str = typer.Option("http://localhost:8002", "--monitor-url", "-m", help="Monitor base URL"),
monitor_url: Optional[str] = typer.Option(None, "--monitor-url", "-m", help="Monitor base URL"),
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the fix! It would be good to add to the help message "Overwrites SWF_MONITOR_URL".



# Connect to ActiveMQ
self.conn.connect(
Copy link
Member

Choose a reason for hiding this comment

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

Ah, now I get it! This part is implemented in the run() of the base agent, that is why it is missing here. Thanks for the catch!
It would be cleaner to abstract the connection with the message queue in a method of the base agent, and use it in both run() and here. We can discuss it with @wenaus.

Copy link
Member

Choose a reason for hiding this comment

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

Will address it in a upcoming PR, following the discussion with @buddhasystem

@michmx
Copy link
Member

michmx commented Nov 25, 2025

Adding a few TODO following the discussion

@michmx michmx merged commit 30a9376 into BNLNPPS:main Nov 25, 2025
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.

2 participants