http-add-on
http-add-on copied to clipboard
fix: Rename keda-http-add-on-interceptor-proxy to keda-add-ons-http-interceptor-proxy
Renamed keda-http-add-on-interceptor-proxy to keda-add-ons-http-interceptor-proxy to align with naming conventions as referenced in the source: https://github.com/kedacore/charts/blob/main/http-add-on/Chart.yaml#L3
Checklist
- [x] Commits are signed with Developer Certificate of Origin (DCO)
- [ ] Changelog has been updated and is aligned with our changelog requirements
- [x] Any necessary documentation is added, such as:
README.md- The
docs/directory - The docs repo
Fixes #
Thanks for the PR 😄
This behaviour is documented here: https://github.com/kedacore/http-add-on/blob/main/docs/walkthrough.md#routing-to-the-right-service
Personally I'd prefer to wait until going to GA, but maybe @zroubalik and @tomkerkhove have other opinion
I was doing a TUTORIAL today and thought I couldn't connect, but now that you mention it I understand everything.
Should we abandon the PR then?
I think users will be happier if it is merged because it doesn't actually work regardless of the warnings, but it depends on the project's policy, so if it is not acceptable, it can be discarded.
We should use one naming, regardles what was the installation method, but I would use keda-http-add-on-interceptor-proxy not ...add-ons... so we follow the name of the repo.
@zroubalik Is this correct? I would love to reduce the number of people who stumble across this wonderful add-on tutorial by one as soon as possible.
thank you @kahirokunn, I like your PR very much!
tbh, I also got a bit confused initially by the discrepancy in naming. I took that as a quirky behaviour of the chart and accepted that as is, but unifying the names regardless of the installation method would make me happier :)
Thank you LGTM!
I see the benefit of unifying them, but I'm afraid about the impact. Currently, that service is the touchpoint that users use for their ingresses and changing it will break all the integrations, IMHO we should wait until the going to GA change (which will include more breaking changes for sure) or at least until the dedicated ScalingSet are available (as the users can deploy a new ScalingSet with its own service that won't change). If I'm the only one who see a problem with this, we can merge it and that's all, I can be overthinking xD
I see the benefit of unifying them, but I'm afraid about the impact. Currently, that service is the touchpoint that users use for their ingresses and changing it will break all the integrations, IMHO we should wait until the going to GA change (which will include more breaking changes for sure) or at least until the dedicated ScalingSet are available (as the users can deploy a new ScalingSet with its own service that won't change). If I'm the only one who see a problem with this, we can merge it and that's all, I can be overthinking xD
I see the point, but I think the sooner we do the change the lesser impact, also this is still in beta, so we can do any breaking changes. Users will just need to migrate to the new version.
I see the point, but I think the sooner we do the change the lesser impact
Yes, totally. My point is that ScalingSet feature can offer an smooth migration path as they can just deploy their new ScalingSet and migrate the workloads to it as they can have both in parallel. Currently, they need to deploy another HTTP Add-on (all the components) temporally. This have been as it is for a long, I think that a single extra version isn't a problem. Other option can be just maintaining the helm chart as it is (or even keeping both names) to not break integrations, deploy using make is something introduced as part of 0.6.0 IIRC and I don't think that it has a huge based of users
Yes, so that's another incetive for you to complete ScalingSets ASAP :)) So let's try to merge this once ScalingSets are completed. I think the benefits are clera, let's use same naming for all methods etc.
As I mentioned, I would prefer if we use keda-http-add-on-... prefix instead of the keda-add-ons-http-... so it is aligned with the repo name and also the plural in add-ons doesn't much make sense.
I would not keep both names in the charts, they would become too complex and confusing imho.
@JorTurFer how does that sound to you?
Yes, so that's another incetive for you to complete ScalingSets ASAP :))
lol, no pressure xD
So let's try to merge this once ScalingSets are completed
My point is that ScalingSets feature has to be merged and released (so this PR can be merged for v0.10.0 or so)
As I mentioned, I would prefer if we use keda-http-add-on-... prefix instead of the keda-add-ons-http-... so it is aligned with the repo name and also the plural in add-ons doesn't much make sense.
I prefer this name too, I don't like the other naming at all, but 🤷 xD
Yes, so that's another incetive for you to complete ScalingSets ASAP :))
lol, no pressure xD
Are you sure? 😄
Go Bold 👍
Hi 🤚
Hi 🤚
Hi!
apologies for the slow responses, given the chart might be actually more frequently used and I am not aware of a way to easily and without breaking changes rename the chart, I'm wondering if the rename shouldn't be the other way around. When I look at the diff it performs keda-add-ons-http-interceptor-proxy -> keda-http-add-on-interceptor-proxy (despite the title in the PR)
Hi 🤚
Hi! apologies for the slow responses, given the chart might be actually more frequently used and I am not aware of a way to easily and without breaking changes rename the chart, I'm wondering if the rename shouldn't be the other way around. When I look at the diff it performs
keda-add-ons-http-interceptor-proxy->keda-http-add-on-interceptor-proxy(despite the title in the PR)
Not important rant - I don't personally like the plural version (add-ons) :))
@wozniakjan Sorry, I fixed it.
How about now?
Bump up 👀
I have authored tutorials and documentation for KEDA on multiple occasions, and in doing so, I have found the inconsistencies in its name to be quite troublesome. I fear that allowing this situation to continue will only serve to hinder KEDA’s growth. Above all, I truly hope for a future in which this issue no longer exists, and I am committed to making that vision a reality.
@kahirokunn thank you very much for your efforts, it's greatly appreciated. After some user discovery, I came to the conclusion that it would be now better to rename the other way and follow the convention set up by the helm chart. It looks like majority of the users install this through the chart and changing the service name would require manual effort to rewrite all Ingress objects for them.
Unless @JorTurFer and @zroubalik object, I would like to propose going this way
keda-http-add-on-interceptor-proxy -> keda-add-ons-http-interceptor-proxy
@wozniakjan I agree with this proposal. While naming conventions are certainly important, consistency is paramount. When there is consistency, confusion is eliminated. If @JorTurFer and @zroubalik are in agreement, I am willing to work on creating a new PR. I would greatly appreciate hearing both of your thoughts on this matter.
Yeah, agreed with @wozniakjan
Thank you.
I have standardized all instances of keda-http-add-on-interceptor-proxy to keda-add-ons-http-interceptor-proxy.
I would appreciate your review.
Thx 🙏
Thank you 🙏 I also dealt with the docs you pointed out. I think I've dealt with everything now, but what do you think?