Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[🐛 BUG]: Config for RPC plugin does not work when included from another file #2017

Closed
1 task done
Kaspiman opened this issue Oct 5, 2024 · 10 comments · Fixed by #2063
Closed
1 task done

[🐛 BUG]: Config for RPC plugin does not work when included from another file #2017

Kaspiman opened this issue Oct 5, 2024 · 10 comments · Fixed by #2063
Assignees
Labels
B-bug Bug: bug, exception F-need-verification
Milestone

Comments

@Kaspiman
Copy link

Kaspiman commented Oct 5, 2024

No duplicates 🥲.

  • I have searched for a similar issue in our bug tracker and didn't find any solutions.

What happened?

When splitting configs, forwarding the rpc.listen field or something like that doesn't work. The error says that the address is not specified, although I assume that the plugin did not start or the config cannot be read.

Version (rr --version)

rr version 2024.2.1 (build time: 2024-09-12T16:25:02+0000, go1.23.1), OS: linux, arch: amd64

How to reproduce the issue?

.rr.base.yaml:

version: '3'

rpc:
    listen: tcp://127.0.0.1:6001

# also 0.0.0.0 does not working

.rr.yaml:

version: '3'

include:
    - .rr.base.yaml
...

then run rr workers

Relevant log output

/var/www/html # rr workers
rpc service not specified in the configuration. Tip: add
 rpc:
 listen: rr_rpc_address
@rustatian
Copy link
Member

Hey @Kaspiman 👋
The case is, that RR commands (like ./rr workers) are using your initial configuration. Replace works for the plugins, but not for the RR commands. This is not a bug, but unexpected expected (huh) behavior. I'll put this to the 2024.3 milestone and add support for the replaces in the RR commands as well.

@rustatian rustatian added this to the v2024.3 milestone Oct 5, 2024
@rustatian rustatian moved this to 🔖 Ready in Jira 😄 Oct 5, 2024
@nickdnk
Copy link
Member

nickdnk commented Oct 8, 2024

As discused in Discord, this is a problem for me as well. I'm loading different paths/routes in my application depending on domain, which requires separate RR nodes. To simplify config, I use a parent config with this:

version: "3"
rpc:
  listen: tcp://127.0.0.1:6010
kv:
  roadrunner:
    driver: memory
    config: {}
http:
  address: 0.0.0.0:8080
  pool:
    allocate_timeout: 60s
    num_workers: 2
logs:
  level: debug
  mode: development

And a child config like

version: "3"
server:
  command: "php app-with-domain-specific-routes.php"
include:
  - .rr.yaml

But loading the kv plugin like this doesn't work, and gives:

ERROR kv  can't find local or global configuration, this section will be skipped  {"local": "kv.roadrunner.config", "global": "roadrunner"}

Having rpc in the parent config does work for me though.

@Kaspiman
Copy link
Author

@rustatian Hi, maybe you have a little time for this bug?

@rustatian
Copy link
Member

@Kaspiman Yes, sure, currently working on it.

@rustatian
Copy link
Member

Ok, original case was fixed. The case with the KV plugin is slightly more complicated, since yaml (God bless this technology (sarcasm)), if the key is empty (e.g.: config:{}) treating this while parsing as - no key. This is the reason, why the KV plugin doesn't see this particular driver - memory (there is no config for it). If you add some nonsense configuration to it, like:
image
Everything would work as expected.

@rustatian
Copy link
Member

Yeah, it is empty:
image
I think it should be handled in the driver.

@rustatian
Copy link
Member

Ok, @nickdnk , I'll update the docs to handle this case with the memory driver, without forking viper, I can't find a way to get this key.

@nickdnk
Copy link
Member

nickdnk commented Nov 21, 2024

You could consider just not requiring the config object at all since it doesn't do anything. Of course, this means if-else'ing where you load drivers to check if the type is kv memory.

@rustatian
Copy link
Member

Yeah, and that what I'm trying to avoid. To have some special behavior for different drivers. I'm thinking about just mentioning this in docs, but, you know, this is not a super expected behavior.

@rustatian
Copy link
Member

Ok, I've resolved this case with the in-memory KV plugin. Drivers now are solely responsible for the configuration parsing and sending errors in case of the empty configuration. Since there is no configuration at all in the in-memory plugin, it'll send a warning, that key was not found, but it'll be running since it does not require configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-bug Bug: bug, exception F-need-verification
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants