Skip to content

chore: change build producer in build + fix lint #328

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.

Sign up for GitHub

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

Merged
shimib merged 4 commits into from
Jul 24, 2026
Merged

chore: change build producer in build + fix lint #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4ae060a
chore(dep): drop local replace for producer
zdtsw Jul 20, 2026
883810a
chore(lint): exclude sortedset_impl.go from gochecknoglobals
zdtsw Jul 20, 2026
f38da95
chore(deps): use go.work to point to api and pipeline instead of replace
zdtsw Jul 21, 2026
c347f75
update: code review
zdtsw Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ linters:
linters: [gochecknoglobals]
- path: pkg/pubsub/pubsubimpl\.go
linters: [gochecknoglobals]
- path: pkg/redis/redisimpl\.go
- path: pkg/redis/(redisimpl|sortedset_impl)\.go
linters: [gochecknoglobals]
Comment thread
zdtsw marked this conversation as resolved.
- path: pkg/server/(options|runner)\.go
linters: [gochecknoglobals]
Expand Down
7 changes: 5 additions & 2 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ WORKDIR /workspace
# Copy the Go Modules manifests
COPY go.mod go.mod
COPY go.sum go.sum
# Local replace directives (./api, ./pipeline, ./producer) must exist before go mod download.
# go.work redirects ./api and ./pipeline to local source; both dirs must exist before go mod download.
# producer is in the workspace for local dev/e2e but is not imported by cmd/main.go, so drop it here
# to avoid copying producer/ into the build context.
COPY go.work go.work
COPY api/ api/
COPY pipeline/ pipeline/
COPY producer/ producer/
RUN go work edit -dropuse ./producer
# cache deps before building and copying source so that we don't need to re-download as much
# and so that source changes don't invalidate our downloaded layer
RUN go mod download
Expand Down
8 changes: 4 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,18 @@ help: ## Display this help.
fmt: ## Run go fmt against root, api, and producer modules.
go fmt ./...
cd api && go fmt ./...
cd producer && go fmt ./...
cd producer && GOWORK=off go fmt ./...

.PHONY: vet
vet: ## Run go vet against root, api, and producer modules.
go vet ./...
cd api && go vet ./...
cd producer && go vet ./...
cd producer && GOWORK=off go vet ./...

.PHONY: test
test: fmt vet setup-envtest ## Run tests (root module and producer submodule).
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out
cd producer && go test ./... -coverprofile=cover-producer.out
cd producer && GOWORK=off go test ./... -coverprofile=cover-producer.out

# Creates a multi-node Kind cluster
# Adds emulated GPU labels and capacities per node
Expand Down Expand Up @@ -388,7 +388,7 @@ set-version:
@echo "Running go mod tidy..."
@go mod tidy
@for d in $(SUBMODULES); do \
if [ -f "$$d/go.mod" ]; then (cd "$$d" && go mod tidy); fi; \
if [ -f "$$d/go.mod" ]; then (cd "$$d" && GOWORK=off go mod tidy); fi; \
done
@echo "Updated cross-module versions:"
@printf " %-20s %-12s %s\n" "SOURCE" "REQUIRES" "VERSION"
Expand Down
15 changes: 3 additions & 12 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,14 @@ module github.com/llm-d/llm-d-async

go 1.25.8

require github.com/llm-d/llm-d-async/api v0.7.4

require github.com/llm-d/llm-d-async/producer v0.7.4

require github.com/llm-d/llm-d-async/pipeline v0.7.4

replace github.com/llm-d/llm-d-async/api => ./api

replace github.com/llm-d/llm-d-async/producer => ./producer

@yizhaodev yizhaodev Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vishbhat
should we also remove those?

replace github.com/llm-d/llm-d-async/api => ./api
replace github.com/llm-d/llm-d-async/pipeline => ./pipeline

@vishbhat vishbhat Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think api and pipeline are being used in some impl. We need it during dev, so that that changes in api and pipeline are immediately picked up.
For producer its fine because the only import is in e2e_test.go

@vishbhat vishbhat Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One item which we might need to handle: test/e2e/e2e_test.go still imports github.com/llm-d/llm-d-async/producer. After this change, it will resolve from the published v0.7.4 module instead of the local ./producer source. This is fine for CI, but if someone is iterating on both producer/ and e2e tests locally, they'll need to temporarily re-add the replace directive to pick up their local changes.

@yizhaodev yizhaodev Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you should remove replace, and use go.work to use local module for development

@yizhaodev yizhaodev Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

go.work will like this, then you don't need hacking go.mod with replace

go 1.25.8

use (
    .
    ./api
    ./pipeline
    ./producer
)

@zdtsw zdtsw Jul 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

correct, but i think this is how was designed at the beginning to make these modulization.

@zdtsw zdtsw Jul 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

a different topic, we should bump go to 1.26 sooner than later

@zdtsw zdtsw Jul 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

changes bit for api and pipeline to use go.work now


replace github.com/llm-d/llm-d-async/pipeline => ./pipeline

require (
cloud.google.com/go/monitoring v1.30.0
cloud.google.com/go/pubsub/v2 v2.6.1
github.com/alicebob/miniredis/v2 v2.38.0
github.com/go-logr/logr v1.4.4
github.com/llm-d/llm-d-async/api v0.7.4
github.com/llm-d/llm-d-async/pipeline v0.7.4
github.com/llm-d/llm-d-async/producer v0.7.4
github.com/onsi/ginkgo/v2 v2.32.0
github.com/onsi/gomega v1.42.1
github.com/prometheus/client_golang v1.24.0
Expand Down
6 changes: 6 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
github.com/llm-d/llm-d-async/api v0.7.4 h1:R3CBDMxlT+9GOyJkgNdQO7zqSpwVFgfz085vSlXn91Q=
github.com/llm-d/llm-d-async/api v0.7.4/go.mod h1:hzjFDTFBJEyW9/1vrHAD2dtCVLTzqggU1GBW+7YhxlQ=
github.com/llm-d/llm-d-async/pipeline v0.7.4 h1:4A2IDYBwgi0tML/3en8zNM16vByZwoKU7L1jaHwxbto=
github.com/llm-d/llm-d-async/pipeline v0.7.4/go.mod h1:G2MztMEFSsWc4bFNyuzwGflt4kagmFKZlXBvarXKvuI=
github.com/llm-d/llm-d-async/producer v0.7.4 h1:dmMNtkorVykYX5UXRYY7qC7q8IesGtbHBY+RFwoaE60=
github.com/llm-d/llm-d-async/producer v0.7.4/go.mod h1:nHxbBM/mVL7bTu2ZVZ+OMz/Up9m2PhpBmeLfoq899+0=
github.com/mailru/easyjson v0.9.0 h1:PrnmzHw7262yW8sTBwxi1PdJA3Iw/EKBa8psRf7d9a4=
github.com/mailru/easyjson v0.9.0/go.mod h1:1+xMtQp2MRNVL/V1bOzuP3aP8VNwRW55fQUto+XFtTU=
github.com/maruel/natural v1.1.1 h1:Hja7XhhmvEFhcByqDoHz9QZbkWey+COd9xWfCfn1ioo=
Expand Down
8 changes: 8 additions & 0 deletions go.work
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
go 1.25.8

use (
.
./api
./pipeline
./producer
)
Loading