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

Expose natsjskv Watcher #142

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

kobergj
Copy link
Contributor

@kobergj kobergj commented Jul 16, 2024

Adds a Watch method to the nats-js-kv store. This exposes the underlying natsjs watcher and allows reacting to changes in the store.

@kobergj
Copy link
Contributor Author

kobergj commented Jul 16, 2024

Pipelines broken unrelated?

go: open /home/runner/work/plugins/plugins/v5/acme/certmagic/go.mod: no such file or directory

@kobergj kobergj requested a review from butonic July 16, 2024 13:46
@kobergj
Copy link
Contributor Author

kobergj commented Jul 23, 2024

@asim is this something we would like to be merged?

@kobergj kobergj marked this pull request as draft July 23, 2024 09:43
@asim asim marked this pull request as ready for review July 23, 2024 10:36
@asim
Copy link
Member

asim commented Jul 23, 2024

If the test pass and it's been go fmt then sure, why not

@kobergj kobergj force-pushed the NatsjskvWatcher branch 2 times, most recently from 89027d7 to 4bc93ff Compare July 24, 2024 10:27
@kobergj
Copy link
Contributor Author

kobergj commented Jul 24, 2024

If the test pass and it's been go fmt then sure, why not

@asim tests seem broken due to v5 update. Not sure what is going wrong. Unrelated to this PR:

go: open /home/runner/work/plugins/plugins/v5/acme/certmagic/go.mod: no such file or directory

@asim
Copy link
Member

asim commented Jul 24, 2024

I think the acme dir is gone?

@kobergj
Copy link
Contributor Author

kobergj commented Jul 25, 2024

I think the acme dir is gone?

I do not know to be honest. I was not involved in v5 update and also didn't see any PRs. So I do not know what the change between v4 and v5 is and neither do I know how this affects v4 builds.

@asim
Copy link
Member

asim commented Jul 25, 2024

I don't know what you're doing in your branch but you need to merge main otherwise this won't work.

@kobergj
Copy link
Contributor Author

kobergj commented Jul 25, 2024

It is up-to-date with main. Previous commit before this one is cc7e2f8

@asim
Copy link
Member

asim commented Jul 25, 2024

I'm not near a computer but I'd do 'grep -r plugins/v5/acme' to see what's referencing this non existent path.

@Gambitier
Copy link

@asim I am having same issue. Other ones are also missing

./v5/acme/certmagic
./v5/codec/bsonrpc
./v5/sync/consul
./v5/sync/etcd
./v5/sync/memory

If the test pass and it's been go fmt then sure, why not

@asim tests seem broken due to v5 update. Not sure what is going wrong. Unrelated to this PR:

go: open /home/runner/work/plugins/plugins/v5/acme/certmagic/go.mod: no such file or directory

@asim
Copy link
Member

asim commented Jul 25, 2024

So it's a go work issue then. Fixing in a separate PR #146

Signed-off-by: jkoberg <jkoberg@owncloud.com>
@kobergj
Copy link
Contributor Author

kobergj commented Jul 30, 2024

There still seems to be something wrong:

go: downloading github.com/Microsoft/go-winio v0.5.2
     | pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies

I tried go mod tidy but it doesn't help.

@asim
Copy link
Member

asim commented Jul 30, 2024

Try delete and reinitialise go work

Signed-off-by: jkoberg <jkoberg@owncloud.com>
@kobergj
Copy link
Contributor Author

kobergj commented Jul 30, 2024

Seems to be even more broken after the go work sync.

go: github.com/micro/go-micro@v1.18.0 requires
	github.com/micro/protoc-gen-micro@v1.0.0: reading github.com/micro/protoc-gen-micro/go.mod at revision v1.0.0: git ls-remote -q origin in /home/runner/go/pkg/mod/cache/vcs/28cfbca4f5e41691076780b2017d5a1000b007b1d3d37b7a9d3b562c7adc9377: exit status 128:
	fatal: could not read Username for 'https://github.com/': terminal prompts disabled

I think this is somehow related to the v5 update. Pipelines were green before it. Did we change build mechanics with v5?

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
do not try to unmarshal on deletes
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