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

Doesn't create a transport.json file #556

Closed
HawtDogFlvrWtr opened this issue Sep 22, 2016 · 12 comments · Fixed by #639
Closed

Doesn't create a transport.json file #556

HawtDogFlvrWtr opened this issue Sep 22, 2016 · 12 comments · Fixed by #639
Milestone

Comments

@HawtDogFlvrWtr
Copy link
Contributor

It would seem that the latest version of the sensu requires a transport.json file, configured for either redis or rabbitmq. I suppose in the past it defaulted to rabbitmq and this file wasn't needed, but now it does.

{ "transport": { "name": "redis", "reconnect_on_error": true } }

@jaxxstorm jaxxstorm added this to the 2.2.0 milestone Oct 23, 2016
@jaxxstorm
Copy link
Contributor

I've added this for 2.2.0 which will be compatible with 0.26. A PR would be appreciated.

@cwjohnston
Copy link
Contributor

@HawtDogFlvrWtr the value of transport name attribute is still expected to default to "rabbitmq" under Sensu 0.26, so I'm not sure why this change should be necessary. Can you please provide more context?

@ciinema
Copy link

ciinema commented Oct 27, 2016

@cwjohnston regarding redis communication ?

@cwjohnston
Copy link
Contributor

@ciinema transport configuration is still needed if you wish to override the default transport name from "rabbitmq" to "redis".

@HawtDogFlvrWtr wrote:

I suppose in the past it defaulted to rabbitmq and this file wasn't needed, but now it does.

The default transport name is still "rabbitmq", so, strictly speaking, configuring transport name should not be required when using the RabbitMQ transport. If folks are experiencing otherwise, that seems like a bug.

@ciinema
Copy link

ciinema commented Oct 27, 2016

but when u want to use redis u need a puppet module with the function to create a transportfile.

that is what i understand or im totally wrong ? :)

@cwjohnston
Copy link
Contributor

cwjohnston commented Oct 27, 2016

@ciinema if you want to override the transport defaults, e.g. to use the "redis" transport, then providing that configuration is required. Strictly speaking, it is not necessary that this config live in a file called transport.json but thats a fine place to put it.

Sorry if I've misunderstood what this issue is regarding, but my larger point is that the assertion that implementing puppet code to drop off transport configuration is "required" for using RabbitMQ as the transport is either incorrect or a bug. If its a bug in Sensu itself, we'd like to correct it. 😉

@pdaugavietis
Copy link

FYI found code for this mixed into #405 for at least the transport.json and such

@jaxxstorm
Copy link
Contributor

Do we feel this should be closed? I haven't seen any more reports of issues there.

@raffraffraff
Copy link

I don't think it can be closed.
While it's true that Sensu does not require a transport.json (because it defaults to rabbitmq), we still need sensu-puppet to support the option so that we can choose to override the default transport via transport.json.

As far as I can see, the latest version (2.2.0) provides no way to configure Redis as a transport. I can't see any PRs that provide this, so I've just forked the project and will test out a quick hack I've done. If it works, I'll send a PR.

@raffraffraff
Copy link

raffraffraff commented Dec 12, 2016

My hacky work so far: https://github.com/raffraffraff/sensu-puppet

I'm going to test this for a while. Feel free to have a look and give me a steer in the right direction before I waste anyone's time with a bad pull request.

@raffraffraff
Copy link

@live3d that is a simpler way to do it alright. The reconnect_on_error isn't configurable, but I assume the default 'true' is what most people want anyway.

@jaxxstorm is live3d's approach be likely to be merged? If so, I'll use it. I need to get moving with the Redis transport...

@jaxxstorm
Copy link
Contributor

the tests are failing, so no :) if they pass, then yes

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 a pull request may close this issue.

6 participants