Skip to content
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

Update gomod #97

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Update gomod #97

merged 2 commits into from
Apr 3, 2024

Conversation

MggMuggins
Copy link
Contributor

I want to use the changes from canonical/lxd#13213 in microcloud; however, building microcloud after make update-gomod fails due to the changes in canonical/lxd@0d903b4. The changes in internal/db allow microcluster to compile again.

Alternatively, pass the context down through DB.retry as in MggMuggins@fde3e86. Let me know which is preferable.

Copy link
Contributor

@markylaing markylaing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please update this PR to pass down any context arguments wherever possible? Thanks.

go.mod Outdated Show resolved Hide resolved
internal/db/db.go Outdated Show resolved Hide resolved
@markylaing
Copy link
Contributor

The reason for all of the OpenFGA imports can be shown with go mod why -m github.com/openfga/openfga. This gives:

# github.com/openfga/openfga
github.com/canonical/microcluster/example/api
github.com/canonical/lxd/lxd/response
github.com/canonical/lxd/lxd/db
github.com/openfga/openfga/pkg/storage
  • The example microcluster uses response.Response to re-use LXD's response type.
  • lxd/response imports lxd/db so that it can check for a sentinel error db.ErrAlreadyDefined when resolving a SmartError.
  • lxd/db imports openfga/pkg/storage for the OpenFGADatastore implementation.

I'll try decoupling lxd/response from lxd/db as a first step. This package is used by microcluster, microcloud, and LXD cloud so it should be moved to shared.

@tomponline
Copy link
Member

TIL go mod why thanks!

@tomponline
Copy link
Member

I'll try decoupling lxd/response from lxd/db as a first step. This package is used by microcluster, microcloud, and LXD cloud so it should be moved to shared

Sounds good, perhaps we can use api.StatusErrorf(http.StatusConflict) instead of db.ErrAlreadyDefined.

@markylaing
Copy link
Contributor

I'll try decoupling lxd/response from lxd/db as a first step. This package is used by microcluster, microcloud, and LXD cloud so it should be moved to shared

Sounds good, perhaps we can use api.StatusErrorf(http.StatusConflict) instead of db.ErrAlreadyDefined.

Yeah that's my plan. Also, moving lxd/response to shared will be quite a headache I think. For now to move this PR along I'll just change the error but I'll create an issue to move it properly.

@tomponline
Copy link
Member

@markylaing sounds good.

@markylaing
Copy link
Contributor

With canonical/lxd#13252 the go mod looks like:

module github.com/canonical/microcluster

go 1.22.0

replace github.com/canonical/lxd => github.com/markylaing/lxd v0.0.0-20240403093903-92aceaa46222

require (
	github.com/canonical/go-dqlite v1.21.0
	github.com/canonical/lxd v0.0.0-20240402145259-e51b0a89c188
	github.com/fsnotify/fsnotify v1.7.0
	github.com/google/renameio v1.0.1
	github.com/gorilla/mux v1.8.1
	github.com/olekukonko/tablewriter v0.0.5
	github.com/spf13/cobra v1.8.0
	golang.org/x/sys v0.18.0
	gopkg.in/yaml.v2 v2.4.0
)

require (
	github.com/Rican7/retry v0.3.1 // indirect
	github.com/armon/go-proxyproto v0.1.0 // indirect
	github.com/flosch/pongo2 v0.0.0-20200913210552-0d938eb266f3 // indirect
	github.com/fvbommel/sortorder v1.1.0 // indirect
	github.com/golang/protobuf v1.5.4 // indirect
	github.com/google/go-cmp v0.6.0 // indirect
	github.com/google/uuid v1.6.0 // indirect
	github.com/gorilla/schema v1.3.0 // indirect
	github.com/gorilla/securecookie v1.1.2 // indirect
	github.com/gorilla/websocket v1.5.1 // indirect
	github.com/gosexy/gettext v0.0.0-20160830220431-74466a0a0c4a // indirect
	github.com/inconshreveable/mousetrap v1.1.0 // indirect
	github.com/kr/fs v0.1.0 // indirect
	github.com/kr/pretty v0.3.1 // indirect
	github.com/mattn/go-runewidth v0.0.15 // indirect
	github.com/mattn/go-sqlite3 v1.14.22 // indirect
	github.com/muhlemmer/gu v0.3.1 // indirect
	github.com/pkg/errors v0.9.1 // indirect
	github.com/pkg/sftp v1.13.6 // indirect
	github.com/pkg/xattr v0.4.9 // indirect
	github.com/rivo/uniseg v0.4.7 // indirect
	github.com/rogpeppe/go-internal v1.10.0 // indirect
	github.com/sirupsen/logrus v1.9.3 // indirect
	github.com/spf13/pflag v1.0.5 // indirect
	github.com/zitadel/oidc/v2 v2.12.0 // indirect
	golang.org/x/crypto v0.21.0 // indirect
	golang.org/x/net v0.22.0 // indirect
	golang.org/x/oauth2 v0.18.0 // indirect
	golang.org/x/sync v0.6.0 // indirect
	golang.org/x/term v0.18.0 // indirect
	golang.org/x/text v0.14.0 // indirect
	google.golang.org/appengine v1.6.8 // indirect
	google.golang.org/protobuf v1.33.0 // indirect
	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
	gopkg.in/square/go-jose.v2 v2.6.0 // indirect
)

@tomponline
Copy link
Member

that has been merged now

Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
`query.Retry` takes a context as of 8e4564c

This allows microcluster to build without changing the behavior of
`DB.Transaction` or `DB.retry`.

Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
@MggMuggins
Copy link
Contributor Author

MggMuggins commented Apr 3, 2024

Thanks all; reran make gomod-update, rebased and ready to go

Copy link
Contributor

@masnax masnax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks!

@masnax masnax merged commit 257dd2e into canonical:main Apr 3, 2024
3 checks passed
@MggMuggins MggMuggins deleted the update-gomod branch April 3, 2024 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants