[v2] Consolidate apiHost & host for RabbitMQ scaler

[turbaszek]

Signed-off-by: Tomek Urbaszek [email protected]

Consolidate apiHost & host for RabbitMQ scaler

Checklist

• Commits are signed with Developer Certificate of Origin (DCO)
• Tests have been added
• A PR is opened to update the documentation on https://github.com/kedacore/keda-docs
• Changelog has been updated

Relates to #711

[turbaszek]

@tomkerkhove @zroubalik can you please take a look? Eventually, do we have a RabbitMQ expert?

[ahmelsayed]

since this is a breaking change for v2, can you also update the CHANGELOG

[tomkerkhove]

Good point! And our migration guide in the docs as well please.

[tomkerkhove]

Looks like we only need a PR for it to https://github.com/kedacore/keda-docs

[zroubalik]

@holyketzer @tbickford could you please take a look on this?

[turbaszek]

Looks like we only need a PR for it to https://github.com/kedacore/keda-docs

Yeah, I will do in the evening

[turbaszek]

@tomkerkhove do you think we need protocol specification? I think we may just resolve it using the host?

[turbaszek]

Docs update: kedacore/keda-docs#253

[zroubalik]

With this change, if http protocol is used, unacked messages are always included and it is no longer optional as it was before. Am I missing anything?

[turbaszek]

Hm, I think this name was confusing. From what I understand the HTTP protocol (apiHost) was used only when includeUnacked==True otherwise this parameter was unused (amqp case):

includeUnacked bool // if true uses HTTP API and requires apiHost, if false uses AMQP and requires host

[turbaszek]

I've done it, I think it's much more userfriendly to just provide host than provide host and partial information from it

[tomkerkhove]

@tomkerkhove do you think we need protocol specification? I think we may just resolve it using the host?

I think protocolis a great addition because maybe today you can figure it out based on the host but:

1. It's a lot easier for people to understand what is being used if they are not a KEDA expert
2. We are ready in case we want to add new ones later on

[turbaszek]

1. It's a lot easier for people to understand what is being used if they are not a KEDA expert

Then using s single uri is the simplest thing I think.

2. We are ready in case we want to add new ones later on

The protocol which is defined by a schema of the host can be determined automatically. And as long as the schema is provided (probably always) we should be able to figure it out for users.

If we agree to have protocol param then I will revert the last commit

[tomkerkhove]

I would prefer to keep it because if you don't use a literal value in the scaler metadata you have to go check somewhere else. From a ops/management team I prefer not to have to do that.

[turbaszek]

I would prefer to keep it because if you don't use a literal value in the scaler metadata you have to go check somewhere else.

That's a good point

[turbaszek]

Removed the last commit, please take a look at the code as well as the related docs PR: kedacore/keda-docs#253

[zroubalik]

Thanks!