Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Upgrades this repo’s Go toolchain and CI environment, and adjusts TLDB DSN parsing to avoid a Go 1.26 net/url.Parse behavior change that rejects sqlite3://:memory:.
Changes:
- Bump Go version references to 1.26.0 (go.mod + CI workflows + project docs).
- Update CI PostGIS service image from PG12 to PG16.
- Replace
url.Parseusage in TLDB DSN handling with string-splitting to preserve support forsqlite3://:memory:; remove stale top-level Dockerfile.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tldb/common.go |
Reworks getFvids to parse query params without url.Parse to avoid Go 1.26 DSN rejection. |
tldb/adapter.go |
Reworks adapter selection to derive scheme without url.Parse. |
go.mod |
Updates Go version directive to 1.26.0. |
Dockerfile |
Removes an obsolete, EOL-pinned Dockerfile. |
.github/workflows/test.yml |
Updates Go version and PostGIS service image for CI. |
.github/workflows/release.yml |
Updates Go version in release build matrices. |
.github/workflows/check-go-generate.yaml |
Updates Go version for codegen checks. |
.claude/CLAUDE.md |
Updates documented Go version reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or 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
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.
Summary
Upgrades the Go toolchain from 1.24.2 to 1.26.0, bumps the CI PostGIS image from EOL PG 12 to PG 16 to match production, removes a stale Dockerfile, and carves out an explicit exception for the library's
sqlite3://:memory:DSN which Go 1.26's stricternet/url.Parsenow rejects.Go toolchain bump
go.moddirective bumped togo 1.26.0.github/workflows/test.yml,.github/workflows/check-go-generate.yaml, and all three build matrices in.github/workflows/release.yml(linux, macos-intel, macos-apple) set togo-version: '1.26.0'.claude/CLAUDE.mdproject-overview line updated to reflect Go 1.26CI PostgreSQL image bump
.github/workflows/test.ymltest service image bumped frompostgis/postgis:12-3.4-alpinetopostgis/postgis:16-3.4-alpine. PostgreSQL 12 reached EOL in Nov 2024; PG 16 more closely matches production. The library uses no PG 13+ exclusive SQL (noMERGE,JSON_TABLE,ANY_VALUE, etc.), so the CI-only behavior change should be minimal. PostGIS pinned at 3.4 to keep scope tight.Dockerfile cleanup
Dockerfile, which had been untouched since 2020 and was pinned to Ubuntu 18.04 (EOL) andgolang-1.12via a third-party PPA. It was not referenced anywhere in CI.net/url regression in Go 1.26
Go 1.26 tightened
net/url.Parseand now rejectssqlite3://:memory:because:memory:after//parses ashost:portwith a non-numeric port. This brokeTestManager_*tests ininternal/feedstate/and any library consumer using the same in-memory DSN form.Fix: keep
url.Parseas the primary mechanism (preserving fragment handling, scheme case-normalization, and structural validation) and add an explicit string-equality carve-out for the one known incompatible form, shared astldb.sqliteMemoryDBURL:tldb/adapter.go—newAdaptershort-circuits the sqlite memory DSN and returns the registeredsqlite3adapter directly; all other dburls go throughurl.Parseunchangedtldb/common.go—getFvidsreturns an empty fvid list for the sqlite memory DSN and otherwise parses as before; errors fromurl.Parsecontinue to propagateExisting tests in
internal/feedstate/manager_test.goandtldb/sqlite/sqlite_test.goexercise this DSN end-to-end, so no new tests were added. ThegetFvidsURL-smuggling behavior is preserved in case external consumers of transitland-lib rely on it.Workflow hygiene
.github/workflows/check-go-generate.yaml— bumpedactions/github-script@v6to@v7for consistency withrelease.ymlTest plan
go versionreports 1.26.x locally before buildinggo mod tidyproduces no diffgo vet ./...is clean(cd cmd/transitland && go build .)succeedsgo test ./internal/feedstate/... ./tldb/...passes (previously failing on 1.26)