thanos
thanos copied to clipboard
thanos-io
Replace go-bindata with a better alternative
Is your proposal related to a problem?
We currently use go-bindata and it is not maintained. Also, another (kind of unrelated) problem is that we capture the modification time of files hence we cannot run make assets on each build otherwise we'd pollute the commits.
Describe the solution you'd like
Prometheus and other related projects use vfsgen: https://github.com/prometheus/prometheus/issues/2411
I suggest moving to it. Plus, let's use a consistent modification time: https://github.com/prometheus/prometheus/pull/4731/files.
Describe alternatives you've considered
Not moving to anything at all and having the same issues. Relevant blog post: https://tech.townsourced.com/post/embedding-static-files-in-go/.
I believe there might be a better way: Go 1.6 has a native solution for this: https://golangtutorial.dev/tips/embed-files-in-go/ go:embed
WDYT? 🤗
Hey, can I work on this issue? :)
Sure, feel free. The embed package looks great, we should try it.
The embed package looks interesting but there is a slight difference in the approach with the embed package. With go-bindata you create the bindata.go file before running go build, which means that you don't need the actual files to successfully build the binary, you just need the generated bindata.go file. But go:embed directives work during the build phase, this means that every time you build the binary, you will need the React UI's production bundle to be available.
So essentially, if we switch to go embed, the build step will also include building the UI (which means one needs to have Node.js installed). On the flip side, this will remove the need of running make assets on each change to the UI and get rid of the merge conflicts in bindata.go, resulting in better DX. BTW Prometheus also does this now (building the UI too when you run make build).
WDYT?
The
embedpackage looks interesting but there is a slight difference in the approach with theembedpackage. Withgo-bindatayou create thebindata.gofile before runninggo build, which means that you don't need the actual files to successfully build the binary, you just need the generatedbindata.gofile. Butgo:embeddirectives work during the build phase, this means that every time you build the binary, you will need the React UI's production bundle to be available.So essentially, if we switch to go embed, the build step will also include building the UI (which means one needs to have Node.js installed). On the flip side, this will remove the need of running
make assetson each change to the UI and get rid of the merge conflicts inbindata.go, resulting in better DX. BTW Prometheus also does this now (building the UI too when you runmake build).WDYT?
It sounds great. So I just went ahead and make a PR. Although there's still a blocker from upstream Prometheus image not bumping to 1.16 yet.
Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.
![stale[bot] avatar](https://avatars.githubusercontent.com/in/1724?v=4 "Published by a pub.dev verified publisher"){kind=small}
Closing for now as promised, let us know if you need this to be reopened! 🤗
Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.
Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.
Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.
Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.
Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.
@onprem how does opening a PR here work? first PR first serve or the team assigns the issue to a person and get to work on it exclusively?
After tracking through some of the open issues and closed PR requests in the prometheus repo, they looked like they moved away from vsfgen to embed.
https://github.com/prometheus/prometheus/pull/10220
Will using embed be a better alternative now?
