-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup usage of go.rice in favor of go:embed #10956
Merged
GuptaManan100
merged 4 commits into
vitessio:main
from
planetscale:dbussink/remove-go-rice
Aug 9, 2022
Merged
Cleanup usage of go.rice in favor of go:embed #10956
GuptaManan100
merged 4 commits into
vitessio:main
from
planetscale:dbussink/remove-go-rice
Aug 9, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
mdlayher
approved these changes
Aug 8, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, solid cleanup.
The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
dbussink
force-pushed
the
dbussink/remove-go-rice
branch
from
August 9, 2022 07:08
667391d
to
16c6431
Compare
dbussink
changed the title
WIP: Cleanup usage of go.rice in favor of go:embed
Cleanup usage of go.rice in favor of go:embed
Aug 9, 2022
dbussink
requested review from
notfelineit,
rohit-nayak-ps,
ajm188,
deepthi,
shlomi-noach,
GuptaManan100 and
mattlord
as code owners
August 9, 2022 07:09
GuptaManan100
approved these changes
Aug 9, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
dbussink
added a commit
to dbussink/vitess
that referenced
this pull request
Aug 9, 2022
Accidentally added in vitessio#10956 Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
3 tasks
ajm188
pushed a commit
that referenced
this pull request
Aug 10, 2022
Accidentally added in #10956 Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
systay
pushed a commit
to planetscale/vitess
that referenced
this pull request
Aug 19, 2022
…sio#937) * Cleanup usage of go.rice in favor of go:embed The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move vtctld to also use `go embed` instead of go.rice Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove last rice-box related comments Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove config moving This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
timvaillancourt
pushed a commit
to slackhq/vitess
that referenced
this pull request
May 23, 2023
* Cleanup usage of go.rice in favor of go:embed The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move vtctld to also use `go embed` instead of go.rice Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove last rice-box related comments Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove config moving This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
4 tasks
maksimov
pushed a commit
to slackhq/vitess
that referenced
this pull request
May 26, 2023
* Cleanup usage of go.rice in favor of go:embed The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move vtctld to also use `go embed` instead of go.rice Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove last rice-box related comments Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove config moving This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com>
maksimov
pushed a commit
to slackhq/vitess
that referenced
this pull request
Jun 13, 2023
* Cleanup usage of go.rice in favor of go:embed The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move vtctld to also use `go embed` instead of go.rice Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove last rice-box related comments Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove config moving This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
8 tasks
maksimov
added a commit
to slackhq/vitess
that referenced
this pull request
Aug 10, 2023
* Cleanup usage of go.rice in favor of go:embed (vitessio#10956) * Cleanup usage of go.rice in favor of go:embed The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move vtctld to also use `go embed` instead of go.rice Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove last rice-box related comments Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove config moving This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * fix vtrootbin Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the build to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the workflows to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the docker image to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the bootstrap version to get docker to go 1.20.5 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use 1.18.7 for static checks Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * lower the build version requirement to allow static checks to pass Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * experiment with using old and new Go for a single workflow Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use old and new Go for a upgrade/downgrade workflows Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * set -buildvcs=false Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * address build errors Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * Revert "fix parameter name (#93)" (#100) This reverts commit 76159fd. * apply patch 12178 to v14 (#102) Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> * more workflows to use self-hosted runner * even more workflows to use self-hosted runner * partially switch upgrade-downgrade to self-hosted runner * Ejortegau/larger runners (#115) * Empty commit to trigger CI Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5-dsdefense`: use larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use runner group instead Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rename group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move more jobs to runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * use vitess-ubuntu20 runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rever change of runner type for e2e ERS PRS new features heavy test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com> * Revert change of runner type for shardedrecovery_stress_verticalsplit_heavy. (#116) We do this because the test is taking a lot longer & failing in some cases with the larger runner. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5`: use dedicated larger runner (#113) * `slack-vitess-r14.0.5`: use dedicated larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix fileNameFromPosition Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * `slack-vitess-r14.0.5`: allow conn overrides in consul topo (#111) * `slack-vitess-r14.0.5`: allow conn overrides in consul topo Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix e2e test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Roderick Yao <roderick.yao@slack-corp.com> Co-authored-by: pbibra <pbibra@slack-corp.com> Co-authored-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com>
maksimov
added a commit
to slackhq/vitess
that referenced
this pull request
Sep 13, 2023
* Cleanup usage of go.rice in favor of go:embed (vitessio#10956) * Cleanup usage of go.rice in favor of go:embed The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move vtctld to also use `go embed` instead of go.rice Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove last rice-box related comments Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove config moving This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * fix vtrootbin Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the build to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the workflows to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the docker image to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the bootstrap version to get docker to go 1.20.5 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use 1.18.7 for static checks Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * lower the build version requirement to allow static checks to pass Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * experiment with using old and new Go for a single workflow Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use old and new Go for a upgrade/downgrade workflows Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * set -buildvcs=false Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * address build errors Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * Revert "fix parameter name (#93)" (#100) This reverts commit 76159fd. * apply patch 12178 to v14 (#102) Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> * more workflows to use self-hosted runner * even more workflows to use self-hosted runner * partially switch upgrade-downgrade to self-hosted runner * Ejortegau/larger runners (#115) * Empty commit to trigger CI Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5-dsdefense`: use larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use runner group instead Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rename group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move more jobs to runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * use vitess-ubuntu20 runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rever change of runner type for e2e ERS PRS new features heavy test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com> * Revert change of runner type for shardedrecovery_stress_verticalsplit_heavy. (#116) We do this because the test is taking a lot longer & failing in some cases with the larger runner. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5`: use dedicated larger runner (#113) * `slack-vitess-r14.0.5`: use dedicated larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix fileNameFromPosition Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * `slack-vitess-r14.0.5`: allow conn overrides in consul topo (#111) * `slack-vitess-r14.0.5`: allow conn overrides in consul topo Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix e2e test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Roderick Yao <roderick.yao@slack-corp.com> Co-authored-by: pbibra <pbibra@slack-corp.com> Co-authored-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com>
maksimov
added a commit
to slackhq/vitess
that referenced
this pull request
Sep 13, 2023
* Cleanup usage of go.rice in favor of go:embed (vitessio#10956) * Cleanup usage of go.rice in favor of go:embed The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move vtctld to also use `go embed` instead of go.rice Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove last rice-box related comments Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove config moving This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * fix vtrootbin Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the build to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the workflows to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the docker image to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the bootstrap version to get docker to go 1.20.5 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use 1.18.7 for static checks Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * lower the build version requirement to allow static checks to pass Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * experiment with using old and new Go for a single workflow Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use old and new Go for a upgrade/downgrade workflows Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * set -buildvcs=false Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * address build errors Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * Revert "fix parameter name (#93)" (#100) This reverts commit 76159fd. * apply patch 12178 to v14 (#102) Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> * more workflows to use self-hosted runner * even more workflows to use self-hosted runner * partially switch upgrade-downgrade to self-hosted runner * Ejortegau/larger runners (#115) * Empty commit to trigger CI Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5-dsdefense`: use larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use runner group instead Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rename group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move more jobs to runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * use vitess-ubuntu20 runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rever change of runner type for e2e ERS PRS new features heavy test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com> * Revert change of runner type for shardedrecovery_stress_verticalsplit_heavy. (#116) We do this because the test is taking a lot longer & failing in some cases with the larger runner. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5`: use dedicated larger runner (#113) * `slack-vitess-r14.0.5`: use dedicated larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix fileNameFromPosition Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * `slack-vitess-r14.0.5`: allow conn overrides in consul topo (#111) * `slack-vitess-r14.0.5`: allow conn overrides in consul topo Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix e2e test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Roderick Yao <roderick.yao@slack-corp.com> Co-authored-by: pbibra <pbibra@slack-corp.com> Co-authored-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com>
maksimov
added a commit
to slackhq/vitess
that referenced
this pull request
Sep 18, 2023
* Upgrade go slack vitess r14.0.5 (#91) * Cleanup usage of go.rice in favor of go:embed (vitessio#10956) * Cleanup usage of go.rice in favor of go:embed The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move vtctld to also use `go embed` instead of go.rice Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove last rice-box related comments Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove config moving This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * fix vtrootbin Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the build to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the workflows to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the docker image to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the bootstrap version to get docker to go 1.20.5 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use 1.18.7 for static checks Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * lower the build version requirement to allow static checks to pass Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * experiment with using old and new Go for a single workflow Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use old and new Go for a upgrade/downgrade workflows Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * set -buildvcs=false Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * address build errors Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * Revert "fix parameter name (#93)" (#100) This reverts commit 76159fd. * apply patch 12178 to v14 (#102) Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> * more workflows to use self-hosted runner * even more workflows to use self-hosted runner * partially switch upgrade-downgrade to self-hosted runner * Ejortegau/larger runners (#115) * Empty commit to trigger CI Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5-dsdefense`: use larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use runner group instead Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rename group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move more jobs to runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * use vitess-ubuntu20 runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rever change of runner type for e2e ERS PRS new features heavy test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com> * Revert change of runner type for shardedrecovery_stress_verticalsplit_heavy. (#116) We do this because the test is taking a lot longer & failing in some cases with the larger runner. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5`: use dedicated larger runner (#113) * `slack-vitess-r14.0.5`: use dedicated larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix fileNameFromPosition Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * `slack-vitess-r14.0.5`: allow conn overrides in consul topo (#111) * `slack-vitess-r14.0.5`: allow conn overrides in consul topo Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix e2e test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Roderick Yao <roderick.yao@slack-corp.com> Co-authored-by: pbibra <pbibra@slack-corp.com> Co-authored-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com> * Upgrade go slack vitess r14.0.5 (#91) * Cleanup usage of go.rice in favor of go:embed (vitessio#10956) * Cleanup usage of go.rice in favor of go:embed The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need. go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move vtctld to also use `go embed` instead of go.rice Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove last rice-box related comments Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove config moving This right now breaks building the actual tests since the tests might also end up loading the regular code which has a `go embed` and refers to the package with the config embeds. This doesn't mean that the config isn't properly included in the binaries. Also with using `go embed` we have a build time dependency on the files and we always know the latest is included, so we don't have the issue of potentially outdated files either. All in all, it seems simplest to remove this logic and trust that Go itself works as advertised. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * fix vtrootbin Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the build to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the workflows to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the docker image to 1.19.10 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * update the bootstrap version to get docker to go 1.20.5 Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use 1.18.7 for static checks Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * lower the build version requirement to allow static checks to pass Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * experiment with using old and new Go for a single workflow Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * use old and new Go for a upgrade/downgrade workflows Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * set -buildvcs=false Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * address build errors Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> * Revert "fix parameter name (#93)" (#100) This reverts commit 76159fd. * apply patch 12178 to v14 (#102) Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> * more workflows to use self-hosted runner * even more workflows to use self-hosted runner * partially switch upgrade-downgrade to self-hosted runner * Ejortegau/larger runners (#115) * Empty commit to trigger CI Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5-dsdefense`: use larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Use runner group instead Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rename group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Move more jobs to runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * use vitess-ubuntu20 runner group Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Rever change of runner type for e2e ERS PRS new features heavy test Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> --------- Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com> * Revert change of runner type for shardedrecovery_stress_verticalsplit_heavy. (#116) We do this because the test is taking a lot longer & failing in some cases with the larger runner. Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> * `slack-vitess-r14.0.5`: use dedicated larger runner (#113) * `slack-vitess-r14.0.5`: use dedicated larger runner Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix fileNameFromPosition Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * empty commit Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * `slack-vitess-r14.0.5`: allow conn overrides in consul topo (#111) * `slack-vitess-r14.0.5`: allow conn overrides in consul topo Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix e2e test Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Roderick Yao <roderick.yao@slack-corp.com> Co-authored-by: pbibra <pbibra@slack-corp.com> Co-authored-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com> * Cleaning up bad merge * Fixing the build cmds --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: 'Stanislav Maksimov' <smaksimov@slack-corp.com> Signed-off-by: Priya Bibra <pbibra@slack-corp.com> Signed-off-by: 'Priya Bibra' <pbibra@slack-corp.com> Signed-off-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Roderick Yao <roderick.yao@slack-corp.com> Co-authored-by: pbibra <pbibra@slack-corp.com> Co-authored-by: Eduardo J. Ortega U <5791035+ejortegau@users.noreply.github.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The usage of go.rice predates the availability of go:embed, but we should switch to using go:embed instead to ship specific assets like config files that we need.
go.rice is also incompatible with Go 1.19 and while it might see a fix in the future, it seems better to go with the recommended Go approach that is available these days.
Right now is still a WIP since it doesn't remove the usage invtctld
yet where we package the UI assets. Want to see first what the fallout of this looks like on CI.Related Issue(s)
Was also mentioned before in the Go 1.16 upgrade but never tackled since: #7572 (comment)
Checklist