From 8ae336d54f7a6bcd0b5cdaff0ec557f803bc24f2 Mon Sep 17 00:00:00 2001 From: matthewpeterkort Date: Tue, 6 Oct 2026 16:02:26 -0700 Subject: [PATCH 1/2] Fix Gen3 ping probe and upload progress summary --- cmd/ping/main.go | 29 +++++++++++++++++-------- cmd/ping/main_test.go | 27 +++++++++++++++++++++-- cmd/ping/terra_integration_test.go | 2 +- docs/commands.md | 2 +- internal/transfer/progress_push.go | 20 +++++++++-------- internal/transfer/progress_push_test.go | 6 +++++ 6 files changed, 64 insertions(+), 22 deletions(-) diff --git a/cmd/ping/main.go b/cmd/ping/main.go index b8bb096f..0983815b 100644 --- a/cmd/ping/main.go +++ b/cmd/ping/main.go @@ -1,7 +1,9 @@ package ping import ( + "bytes" "context" + "encoding/json" "fmt" "io" "log/slog" @@ -35,8 +37,8 @@ type healthInfo struct { } var pingHealth = func(ctx context.Context, gc *remoteruntime.GitContext) (healthInfo, error) { - if gc != nil && gc.IsReadOnly() { - serviceInfo, err := pingTerraServiceInfo(ctx, gc.Endpoint) + if gc != nil && (gc.IsReadOnly() || gc.RemoteType == config.Gen3ServerType) { + serviceInfo, err := pingDRSServiceInfo(ctx, gc.Endpoint) return healthInfo{ServiceInfo: serviceInfo}, err } return healthInfo{}, gc.Client.Health().Ping(ctx) @@ -76,7 +78,7 @@ var Cmd = &cobra.Command{ } fmt.Println("health: ok") if strings.TrimSpace(health.ServiceInfo) != "" { - fmt.Printf("service-info: %s\n", health.ServiceInfo) + printServiceInfo(health.ServiceInfo) } scopeInfo, err := pingScopeAccess(cmd.Context(), gc) @@ -102,6 +104,15 @@ var Cmd = &cobra.Command{ }, } +func printServiceInfo(raw string) { + var formatted bytes.Buffer + if err := json.Indent(&formatted, []byte(raw), " ", " "); err != nil { + fmt.Printf("service-info: %s\n", raw) + return + } + fmt.Printf("service-info:\n %s\n", formatted.String()) +} + func resolveStatus(args []string, logger *slog.Logger) (statusInfo, *remoteruntime.GitContext, error) { cfg, err := config.LoadConfig() if err != nil { @@ -230,11 +241,11 @@ func checkScopeAccess(ctx context.Context, gc *remoteruntime.GitContext) (scopeA return info, nil } -func pingTerraServiceInfo(ctx context.Context, endpoint string) (string, error) { +func pingDRSServiceInfo(ctx context.Context, endpoint string) (string, error) { if ctx == nil { ctx = context.Background() } - serviceInfoURL, err := terraServiceInfoURL(endpoint) + serviceInfoURL, err := drsServiceInfoURL(endpoint) if err != nil { return "", err } @@ -249,7 +260,7 @@ func pingTerraServiceInfo(ctx context.Context, endpoint string) (string, error) defer resp.Body.Close() serviceInfo, readErr := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - return "", fmt.Errorf("terra DRS service-info returned %s", resp.Status) + return "", fmt.Errorf("DRS service-info returned %s", resp.Status) } if readErr != nil { return "", readErr @@ -257,17 +268,17 @@ func pingTerraServiceInfo(ctx context.Context, endpoint string) (string, error) return strings.TrimSpace(string(serviceInfo)), nil } -func terraServiceInfoURL(endpoint string) (string, error) { +func drsServiceInfoURL(endpoint string) (string, error) { endpoint = strings.TrimSpace(endpoint) if endpoint == "" { - return "", fmt.Errorf("terra endpoint is empty") + return "", fmt.Errorf("DRS endpoint is empty") } parsed, err := url.Parse(endpoint) if err != nil { return "", err } if parsed.Scheme == "" || parsed.Host == "" { - return "", fmt.Errorf("terra endpoint must be an absolute URL: %q", endpoint) + return "", fmt.Errorf("DRS endpoint must be an absolute URL: %q", endpoint) } parsed.Path = strings.TrimRight(parsed.Path, "/") + "/ga4gh/drs/v1/service-info" parsed.RawQuery = "" diff --git a/cmd/ping/main_test.go b/cmd/ping/main_test.go index 63075be6..7f04178c 100644 --- a/cmd/ping/main_test.go +++ b/cmd/ping/main_test.go @@ -66,6 +66,29 @@ func TestPingCmdArgs(t *testing.T) { } } +func TestPingHealthGen3UsesDRSServiceInfo(t *testing.T) { + var requests int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/ga4gh/drs/v1/service-info" { + t.Errorf("unexpected health path: %s", r.URL.Path) + http.NotFound(w, r) + return + } + requests++ + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"gen3-drs"}`)) + })) + t.Cleanup(server.Close) + gc := &remoteruntime.GitContext{RemoteType: config.Gen3ServerType, Endpoint: server.URL} + got, err := pingHealth(t.Context(), gc) + if err != nil { + t.Fatalf("Gen3 DRS service-info ping failed: %v", err) + } + if requests != 1 || got.ServiceInfo != `{"id":"gen3-drs"}` { + t.Fatalf("unexpected Gen3 ping result: requests=%d info=%q", requests, got.ServiceInfo) + } +} + func TestAcceptancePingTerraDRSServer(t *testing.T) { var serviceInfoRequests int terraDRS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -116,7 +139,7 @@ func TestAcceptancePingTerraDRSServer(t *testing.T) { "type: terra", "endpoint: " + terraDRS.URL, "health: ok", - `service-info: {"id":"terra-drs","name":"Terra DRS","type":{"group":"org.ga4gh","artifact":"drs","version":"1.0.0"}}`, + "service-info:\n {\n \"id\": \"terra-drs\"", } { if !strings.Contains(got, want) { t.Fatalf("expected output to contain %q, got %q", want, got) @@ -291,7 +314,7 @@ func TestPingRunEPrintsServiceInfo(t *testing.T) { if _, err := io.Copy(&buf, r); err != nil { t.Fatalf("read stdout: %v", err) } - if got, want := buf.String(), `service-info: {"id":"terra-drs","name":"Terra DRS"}`; !strings.Contains(got, want) { + if got, want := buf.String(), "service-info:\n {\n \"id\": \"terra-drs\",\n \"name\": \"Terra DRS\"\n }"; !strings.Contains(got, want) { t.Fatalf("expected output to contain %q, got %q", want, got) } } diff --git a/cmd/ping/terra_integration_test.go b/cmd/ping/terra_integration_test.go index 6321dded..49778381 100644 --- a/cmd/ping/terra_integration_test.go +++ b/cmd/ping/terra_integration_test.go @@ -18,7 +18,7 @@ func TestIntegrationPingTerraDRSServer(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) t.Cleanup(cancel) - if _, err := pingTerraServiceInfo(ctx, endpoint); err != nil { + if _, err := pingDRSServiceInfo(ctx, endpoint); err != nil { t.Fatalf("ping Terra DRS service-info endpoint %q: %v", endpoint, err) } } diff --git a/docs/commands.md b/docs/commands.md index 550b6c70..11436e46 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -129,7 +129,7 @@ What it checks: - prints the selected remote, remote type, endpoint, scope, bucket, storage prefix, and auth mode - runs a health check against the selected remote -- for Terra DRS remotes, pings the GA4GH DRS service-info endpoint at `/ga4gh/drs/v1/service-info` +- for Gen3 and Terra DRS remotes, pings the GA4GH DRS service-info endpoint at `/ga4gh/drs/v1/service-info` - for Terra/AnVIL TDR-hosted data in production, use `https://data.terra.bio` as the endpoint; Terra also uses DRSHub for DRS URI resolution, but DRSHub is a resolver service rather than the GA4GH DRS service-info host - for scoped Syfon-style remotes, verifies that the configured organization/project and bucket are visible and readable diff --git a/internal/transfer/progress_push.go b/internal/transfer/progress_push.go index 8b31df71..d8453218 100644 --- a/internal/transfer/progress_push.go +++ b/internal/transfer/progress_push.go @@ -22,14 +22,15 @@ type metadataProgress struct { } type UploadProgressRenderer struct { - mu sync.Mutex - base *Renderer - err error - planned bool - plan UploadPlanSummary - metadata metadataProgress - files map[string]*uploadFileProgress - fileOrder []string + mu sync.Mutex + base *Renderer + err error + planned bool + hadUploads bool + plan UploadPlanSummary + metadata metadataProgress + files map[string]*uploadFileProgress + fileOrder []string } func NewUploadProgressRenderer(out io.Writer) *UploadProgressRenderer { @@ -102,6 +103,7 @@ func (r *UploadProgressRenderer) OnUploadPlan(plan UploadPlanSummary) { r.plan = plan r.planned = plan.TotalFiles > 0 + r.hadUploads = r.hadUploads || r.planned r.files = make(map[string]*uploadFileProgress, len(plan.Files)) r.fileOrder = r.fileOrder[:0] for _, file := range plan.Files { @@ -178,7 +180,7 @@ func (r *UploadProgressRenderer) Finish() error { func (r *UploadProgressRenderer) HadUploads() bool { r.mu.Lock() defer r.mu.Unlock() - return r != nil && r.planned + return r != nil && r.hadUploads } func (r *UploadProgressRenderer) Err() error { diff --git a/internal/transfer/progress_push_test.go b/internal/transfer/progress_push_test.go index dc6b906f..869b6e6e 100644 --- a/internal/transfer/progress_push_test.go +++ b/internal/transfer/progress_push_test.go @@ -27,6 +27,9 @@ func TestUploadProgressRendererTTY(t *testing.T) { r.OnUploadProgress(UploadProgressEvent{OID: "oid-2", Path: "b.bin", BytesSoFar: 0, TotalBytes: 100, Phase: UploadProgressUploading}) r.OnUploadProgress(UploadProgressEvent{OID: "oid-2", Path: "b.bin", BytesSoFar: 100, TotalBytes: 100, Phase: UploadProgressCompleted}) r.Finish() + if !r.HadUploads() { + t.Fatal("completed upload plan must remain visible after Finish") + } got := out.String() if !strings.Contains(got, "a.bin [============ ] 50.0% 50 B/100 B") { @@ -79,6 +82,9 @@ func TestUploadProgressRendererMetadataOnly(t *testing.T) { r.OnMetadataProgress(MetadataProgressEvent{Completed: 0, Total: 3, Phase: MetadataProgressRegistering}) r.OnMetadataProgress(MetadataProgressEvent{Completed: 3, Total: 3, Phase: MetadataProgressCompleted}) r.Finish() + if r.HadUploads() { + t.Fatal("metadata-only plan reported a payload upload") + } got := out.String() if !strings.Contains(got, "registering metadata") { From 07d5d779a12386f4f52de0a0f1fd66098d7364d7 Mon Sep 17 00:00:00 2001 From: matthewpeterkort Date: Wed, 7 Oct 2026 12:04:22 -0700 Subject: [PATCH 2/2] Avoid apt mirror stall in Syfon E2E workflow --- .github/workflows/syfon-backend-e2e.yaml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/syfon-backend-e2e.yaml b/.github/workflows/syfon-backend-e2e.yaml index cd9b7c83..af0ed6a8 100644 --- a/.github/workflows/syfon-backend-e2e.yaml +++ b/.github/workflows/syfon-backend-e2e.yaml @@ -62,12 +62,9 @@ jobs: git checkout --detach "${{ steps.syfon-ref.outputs.ref }}" git rev-parse --verify HEAD - - name: Install test prerequisites + - name: Initialize Git LFS working-directory: git-drs - run: | - sudo apt-get update - sudo apt-get install -y git-lfs - git lfs install --local + run: git lfs install --local - name: Run Docker-backed git-drs integration tests working-directory: git-drs