goproxy icon indicating copy to clipboard operation
goproxy copied to clipboard

feat: Export Handlers (req, resp, https)

Open guilhem opened this issue 3 years ago • 8 comments

This is usufull when handlers will be dynamicaly created and removed.

guilhem avatar Jul 15 '21 21:07 guilhem

Can you please explain why you can't use the existing methods to manipulate the handlers?

Making a private field public is risky, as it cannot be undone.

elazarl avatar Jul 16 '21 07:07 elazarl

@elazarl in current implementation, Handlers are manipulated by .Do(, .HijackConnect(, .HandleConnect( and only with an = append(. It makes impossible to remove elements or to know what are elements inside.

What I need is to Add / List and Remove handlers to be able to do it dynamically (at run-time).

guilhem avatar Jul 16 '21 08:07 guilhem

@elazarl an usage example: https://github.com/guilhem/egress-proxy-operator/blob/master/controllers/request_controller.go#L126

guilhem avatar Jul 27 '21 16:07 guilhem

I don't see why you need indirection. Making them public should be good enough.

Slice is already a pointer.

Also not sure how you handle concurrency.

elazarl avatar Jan 31 '22 16:01 elazarl

I don't see why you need indirection. Making them public should be good enough.

Slice is already a pointer.

Also not sure how you handle concurrency.

I totally agree with you. But I faced a lot of problems with slices and make it working with pointer. 🤷‍♂️ I need to do more tests.

I also find this article, not sure if applicable: https://medium.com/swlh/golang-tips-why-pointers-to-slices-are-useful-and-how-ignoring-them-can-lead-to-tricky-bugs-cac90f72e77b

guilhem avatar Jan 31 '22 19:01 guilhem

@elazarl I did more tests and slice is not updated if it's not a pointer. Having a pointer also have some interest, it's possible to update content atomically. Just create a new slice with right content and update pointer in 1 step.

guilhem avatar Feb 08 '22 14:02 guilhem