golang: refresh dask-gateway-proxy using modules and package directories#559
Conversation
…-proxy Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
consideRatio
left a comment
There was a problem hiding this comment.
Thank you for your work on this @rigzba21!!! From what I can tell, this looks good. I left a few comments for my own insight but this LGTM from what I can tell.
Also note that the test failures are unrelated at least to this PR (ipython/traitlets#731).
dask-gateway-server/dask-gateway-proxy/cmd/dask-gateway-proxy/main.go
Outdated
Show resolved
Hide resolved
Signed-off-by: rigzba21 <jonathan.velando01@gmail.com>
|
@martindurant @TomAugspurger @jacobtomlinson a ping for review, i figure we merge this soonish without further comments. |
|
I've never written go before, but +1 to the goals of this PR. |
jacobtomlinson
left a comment
There was a problem hiding this comment.
This seems like a great move to me. The only comment I have is why are we putting some code under pkg? I don't think we should be intentionally exposing any public APIs and we should probably place it in internal instead.
I thought about that, however, I figured that might be a design decision outside of the scope of this PR, and might fit better in a separate PR, and I do not have as much context into the long-term goals for dask-gateway-proxy. |
Deal! Let's go for a merge on this. The tests were passing before we ran into an issue I think was related to traitlets (ipython/traitlets#731). |
|
@rigzba21 thank you so much for your work on this!!!!!! 🎉 ❤️ 🌻 |
@consideRatio Restructuring dask-gateway-proxy using go project layout to address #506
Nothing has changed as far as functionality.
router.goandsni.gohave been moved into separate packages under thepkg/directory.main.gohas been moved tocmd/dask-gateway-proxy.logging.gohas been moved to its own internal package underinternal/logging.go.modandgo.sumwith dependencies.pkg/,cmd/dask-gateway-proxy/, andinternal/.pkg/router.I hope this helps!
EDIT: closes #506!