Skip to content

Apply PR #36 review-thread fixes for config path, ClickHouse docs, and test cleanup#40

Draft
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-code-comments-review-36
Draft

Apply PR #36 review-thread fixes for config path, ClickHouse docs, and test cleanup#40
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-code-comments-review-36

Conversation

Copilot AI commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

This addresses the review-thread comments on PR #36 by fixing the default config-file resolution in cmd/server, correcting ClickHouse schema-management documentation, and tightening a shared-state test. It also aligns the RenderTemplate docstring with the actual missing-key behavior.

  • Startup config path

    • Use the existing config-path resolver for the normal app.Run(...) path.
    • Fall back to plain yagpcc.yaml when --config-path is unset, instead of constructing /yagpcc.yaml.
    • Add a focused test covering both the cwd fallback and explicit config directory behavior.
    func runtimeConfigFilePath() string {
    	if path := configFilePath(); path != "" {
    		return path
    	}
    	return configFile
    }
  • ClickHouse schema docs

    • Replace the non-existent --dump-only reference with the implemented CLI/config contract:
      • CLI: --dump-schema
      • Config: schema_management: dump_only
    • Correct the dump_only lifecycle description to match actual behavior: the process exits after printing DDL; it does not continue starting the master.
  • Template API comment accuracy

    • Update RenderTemplate comments to state that missing keys fail with an error, matching template.Option("missingkey=error").
  • Test isolation

    • Restore internal/storage.CurrentTime with t.Cleanup(...) in aggregated_storage_test.go so the package-global override cannot leak into later tests.

Sanikadze and others added 2 commits May 28, 2026 16:41
…on snapshots

Includes a small lifecycle fix in queryCompleted: archive on master
Completed even when not all segments have reported. Without this,
under load garbageCollect evicts queries from RunningQueriesStorage
before the segment-timeout fires, so they never reach archChan and
the CH sink stays empty for query_events / aggregated_metrics.

Closes #34
Copilot AI changed the title [WIP] Fix code based on review comments Apply PR #36 review-thread fixes for config path, ClickHouse docs, and test cleanup Jun 10, 2026
Copilot AI requested a review from leborchuk June 10, 2026 01:07
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.

3 participants