Skip to content

chore: add s2-specs as a submodule#42

Merged
quettabit merged 2 commits intomainfrom
qb/upd-specs
May 1, 2026
Merged

chore: add s2-specs as a submodule#42
quettabit merged 2 commits intomainfrom
qb/upd-specs

Conversation

@quettabit
Copy link
Copy Markdown
Member

No description provided.

@quettabit quettabit marked this pull request as ready for review May 1, 2026 03:45
@quettabit quettabit requested a review from a team as a code owner May 1, 2026 03:45
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 1, 2026

Greptile Summary

This PR converts the previously-tracked s2-specs files into a proper git submodule and introduces an update_specs script to regenerate protobuf bindings from it. The runtime protobuf dependency is correctly bumped to >=6.31.1,<7.0.0 to match the newly generated code, and the generated s2_pb2.py/s2_pb2.pyi files are updated accordingly.

Confidence Score: 5/5

Safe to merge; all substantive dependency and generated-code issues are resolved in this PR

No P0 or P1 findings in the current diff. The previously-flagged P1 (protobuf version mismatch) has been addressed. Only a minor P2 style suggestion on update_specs and the pre-existing types-protobuf stubs version remain.

No files require special attention; pyproject.toml types-protobuf stubs version mismatch is minor and already noted in previous review rounds.

Important Files Changed

Filename Overview
pyproject.toml Runtime protobuf dependency correctly bumped to >=6.31.1,<7.0.0; dev types-protobuf still pinned at a 5.x stub version
update_specs New script to regenerate protobuf bindings from submodule; uses sed -i '' (macOS-only, already discussed) and unquoted xargs pipeline
s2-specs New git submodule pointing to s2-streamstore/s2-specs at commit a47ed3e; .gitmodules correctly configured
src/s2_sdk/_generated/s2/v1/s2_pb2.py Regenerated with protobuf 6.31.1; ValidateProtobufRuntimeVersion call now requires protobuf 6.x, consistent with the dependency bump
src/s2_sdk/_generated/s2/v1/s2_pb2.pyi Updated generated type stubs matching the regenerated s2_pb2.py

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[s2-specs submodule\na47ed3e] -->|s2/v1/s2.proto| B[update_specs script]
    B -->|grpc_tools.protoc| C[s2_pb2.py\ns2_pb2.pyi]
    B -->|sed import fix| C
    B -->|poe cq-fix| D[Linted & formatted generated code]
    C --> D
    D --> E[src/s2_sdk/_generated/s2/v1/]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
update_specs:13
**`find | xargs` pipeline lacks null-delimiter safety**

If any `.py` file path contains spaces or special characters, the plain pipe to `xargs -I{}` will split the path incorrectly, causing `sed` to receive a broken filename. Use `-print0` with `xargs -0` to handle arbitrary filenames safely.

```suggestion
find . -name '*.py' -print0 | xargs -0 -I{} sed -i '' 's/from s2\.\(v[0-9][a-z0-9]*\) import s2_pb2/from s2_sdk._generated.s2.\1 import s2_pb2/' {}
```

Reviews (2): Last reviewed commit: "fix proto dep" | Re-trigger Greptile

Comment thread update_specs
@quettabit
Copy link
Copy Markdown
Member Author

@greptileai review again

@quettabit quettabit merged commit a87ef6b into main May 1, 2026
6 checks passed
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.

1 participant