Conversation
…necessary main test block
There was a problem hiding this comment.
Pull request overview
This PR adjusts the taos-ws-py Rust extension’s upstream Rust connector dependency to a feat/stmt2 branch (presumably to enable/validate stmt2 functionality) and includes minor cleanup in the SQLAlchemy test file.
Changes:
- Update
taos-ws-pyto pulltaosfromtaos-connector-rustbranchfeat/stmt2and refresh the lockfile sources. - Remove a few stale commented lines in
tests/test_sqlalchemy.py.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_sqlalchemy.py | Removes commented-out lines; touched the __main__ section output text. |
| taos-ws-py/Cargo.toml | Switches taos dependency from main to feat/stmt2 branch for unix/windows targets. |
| taos-ws-py/Cargo.lock | Updates git source hashes to the feat/stmt2 branch commit for taos-related crates. |
Comments suppressed due to low confidence (1)
taos-ws-py/Cargo.toml:40
- This switches the
taosgit dependency to track a named branch (feat/stmt2). Branch-based git dependencies can change over time and make builds harder to reproduce/debug. Prefer pinning to an immutable gitrev(or a tag) inCargo.toml, and updateCargo.lockaccordingly if needed.
[target.'cfg(windows)'.dependencies]
taos = { git = "https://github.com/taosdata/taos-connector-rust.git", branch = "feat/stmt2", default-features = false, version = "0.12.3", features = [
"optin",
"ws-rustls",
"ws-rustls-aws-lc-crypto-provider",
] }
[target.'cfg(unix)'.dependencies]
taos = { git = "https://github.com/taosdata/taos-connector-rust.git", branch = "feat/stmt2", default-features = false, version = "0.12.3", features = [
"optin",
"ws-rustls",
"ws-rustls-ring-crypto-provider",
] }
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # main test | ||
| if __name__ == "__main__": | ||
| print("hello, test sqlalcemy db api. do nothing\n") |
There was a problem hiding this comment.
The message printed under __main__ contains a typo: sqlalcemy should be sqlalchemy for clarity (this shows up in test output and logs).
| print("hello, test sqlalcemy db api. do nothing\n") | |
| print("hello, test sqlalchemy db api. do nothing\n") |
There was a problem hiding this comment.
Code Review
This pull request updates the project's Rust dependencies to point to the 'feat/stmt2' branch and cleans up the SQLAlchemy test suite by removing commented-out code. Feedback was provided regarding the risk of using a feature branch as a dependency in 'Cargo.toml', as it may lead to build failures if the branch is modified or deleted; using a specific commit hash or a stable branch is recommended instead.
|
|
||
| [target.'cfg(windows)'.dependencies] | ||
| taos = { git = "https://github.com/taosdata/taos-connector-rust.git", branch = "main", default-features = false, version = "0.12.3", features = [ | ||
| taos = { git = "https://github.com/taosdata/taos-connector-rust.git", branch = "feat/stmt2", default-features = false, version = "0.12.3", features = [ |
There was a problem hiding this comment.
Using a feature branch (feat/stmt2) as a dependency in Cargo.toml is risky for code that might be merged into a stable branch. If the feature branch is deleted or rebased, it can break future builds. It is generally better to point to a specific commit hash or a stable branch like main once the feature is integrated.
| ] } | ||
| [target.'cfg(unix)'.dependencies] | ||
| taos = { git = "https://github.com/taosdata/taos-connector-rust.git", branch = "main", default-features = false, version = "0.12.3", features = [ | ||
| taos = { git = "https://github.com/taosdata/taos-connector-rust.git", branch = "feat/stmt2", default-features = false, version = "0.12.3", features = [ |
There was a problem hiding this comment.
Using a feature branch (feat/stmt2) as a dependency in Cargo.toml is risky for code that might be merged into a stable branch. If the feature branch is deleted or rebased, it can break future builds. It is generally better to point to a specific commit hash or a stable branch like main once the feature is integrated.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #388 +/- ##
=======================================
Coverage 82.00% 82.00%
=======================================
Files 24 24
Lines 3717 3717
=======================================
Hits 3048 3048
Misses 669 669 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
taos-ws-py/Cargo.toml:40
taosdependency is switched to a moving git branch (branch = "feat/stmt2"). Even thoughCargo.lockcurrently pins a commit, the branch reference can change over time and lead to non-reproducible builds when the lockfile is regenerated or when downstream tooling ignores the lock. Prefer pinning to an immutablerev(or a released tag/version on crates.io) for both unix and windows targets, and update the lockfile accordingly.
[target.'cfg(windows)'.dependencies]
taos = { git = "https://github.com/taosdata/taos-connector-rust.git", branch = "feat/stmt2", default-features = false, version = "0.12.3", features = [
"optin",
"ws-rustls",
"ws-rustls-aws-lc-crypto-provider",
] }
[target.'cfg(unix)'.dependencies]
taos = { git = "https://github.com/taosdata/taos-connector-rust.git", branch = "feat/stmt2", default-features = false, version = "0.12.3", features = [
"optin",
"ws-rustls",
"ws-rustls-ring-crypto-provider",
] }
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.