Ping keep-alive error.

[bradyjarvis]

Hello!

I just updated to the latest pull of Chiadog (v0.7.5-3-gc7f2991) I am getting the following error on the keep-alive monitor"

[ ERROR] --- Failed to ping keep-alive: 'Subview' object has no attribute 'type' (keep_alive_monitor.py:116)

Checked the config.yaml and even used the new one and moved over my settings. I did not see anything amiss and there was no change.

[martomi]

Hey, thanks for the report! Did you re-run ./install.sh after pulling? (see readme)

We added a new Python dependency so it’ll be strictly necessary to install dependencies for upgrades. If that doesn’t help, you can go back to the stable release with git checkout v0.7.5.

cc @Artanicus

P.S.: We should improve guides for docker and move everyone to docker-based installs since it’s more secure and dependencies are managed automatically. This always points to the latest stable release: docker pull ghcr.io/martomi/chiadog:latest

[bradyjarvis]

Hello, and thank you for the reply. I did run the ./install.sh script once I pulled the new version. I didn't pay attention to the output, but I will do so and report back.

Thanks again for Chiadog, it is invaluable.

[bradyjarvis]

I did another fetch and pull, then re-ran ./install.sh. It said that all the requirements were satisfied successfully. The issue remains. I will try using the docker version and see if it persists. For now I have disabled the keep-alive ping and everything else seems fine.

I should note that this is on both of my farms. One is running Ubuntu 22.04 LTS and the other is running Ubuntu 22.10. Both installs are having the same issue.

[jinnatar]

I think I know what's going on. The keepalive notifier is getting passed a confuse config instead of a dict, while it still at this revision expects a dict. The keepalive PR would solve it, alternatively I can patch in a quick conversion on the receiving side. On mobile now but will PR that patch as an alternative when I get a chance.

Not sure why type checking or my real world testing didn't catch this, will also try to figure out that flaw and see if anything else is similarly broken. Unittests feed a config forcefully, hence why they could not catch it.

[jinnatar]

Oddly enough I can't replicate the error in my testing, which explains how it got through. Could you please provide a sanitized copy of your config.yaml @bradyjarvis ? I want to make sure existing configs continue to work as-is, so it might be there's some peculiarity there that triggers the issue. Mind you, there is a clear error in the code, but for some reason my test config is not triggering it:

(this has sections the main HEAD does not yet read, but those should not affect things)

notification_title_prefix: 'fullnode'
log_level: DEBUG

chia_logs:
  file_log_consumer:
    enable: true

keep_alive_monitor:
  enable_remote_ping: true
  ping_url: 'https://hc-ping.com/xxx'
  thresholds:
    WALLET: 120

daily_stats:
  enable: true
  time_of_day: 12

monitored_services:
  - FULL_NODE
  - WALLET
  - FARMER

notifier:
  pushover:
    enable: true
    daily_stats: true
    wallet_events: true
    credentials:
      api_token: xxx
      user_key: 'xxx'

[jinnatar]

Actually, it might be just stupid enough that the problem only surfaces when the ping is actually attempted, and I never left it running long enough at the current revision to trigger that. shamecube

Edit: Yes, that was the reason. Now I know better. Type checking should guard against this, but as confuse does not have type stubs published yet, we're a bit vulnerable. (beetbox/confuse#121)

[bradyjarvis]

That fixed it, thanks guys! Appreciate the quick response and glad to have the keep-alive working again.