Migrate BigInt scalar to scalar_map and update dependencies - #260
Conversation
Merging this PR will improve performance by 74.72%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_hello_world |
18.3 µs | 10.5 µs | +74.72% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Ckk3:update-deps-and-fix-bigint-scalar (ce887ed) with main (00d109c)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #260 +/- ##
==========================================
+ Coverage 92.47% 92.50% +0.03%
==========================================
Files 19 20 +1
Lines 2445 2469 +24
Branches 188 185 -3
==========================================
+ Hits 2261 2284 +23
- Misses 98 99 +1
Partials 86 86 🚀 New features to boost your workflow:
|
…awberry-graphql#261) fix release-file-check action
…m/Ckk3/strawberry-sqlalchemy into update-deps-and-fix-bigint-scalar
|
Thanks for adding the Here's a preview of the changelog: Migrate the
Fixes #259. Breaking Changes:
import strawberry
from strawberry.schema.config import StrawberryConfig
from strawberry_sqlalchemy_mapper import strawberry_sqlalchemy_scalar_map
schema = strawberry.Schema(
query=Query,
config=StrawberryConfig(scalar_map=strawberry_sqlalchemy_scalar_map),
)If you already maintain your own config = StrawberryConfig(
scalar_map={**strawberry_sqlalchemy_scalar_map, **my_scalar_map},
)Forgetting to register it raises See Custom scalars in the Strawberry
Fixes:
Internal:
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Fixed security issues:
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/strawberry_sqlalchemy_mapper/scalars.py" line_range="16" />
<code_context>
+ description="BigInt field",
+ serialize=lambda v: int(v),
+ parse_value=lambda v: str(v),
+ ),
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** `parse_value` returns a `str`, but `BigInt` is now declared as `NewType("BigInt", int)`, so GraphQL inputs annotated as `BigInt` are delivered to resolvers as strings despite their type contract. Resolver code performing numeric operations receives string values and can fail or produce incorrect results.
**Triggers:** When a schema uses `BigInt` as an input or argument type and a client supplies a value.
**Suggested fix:** Return `int(v)` from `parse_value` so the runtime value matches the `BigInt`/`int` annotation.
```suggestion
parse_value=lambda v: int(v),
```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: src/strawberry_sqlalchemy_mapper/scalars.py:16
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| name="BigInt", | ||
| description="BigInt field", | ||
| serialize=lambda v: int(v), | ||
| parse_value=lambda v: str(v), |
There was a problem hiding this comment.
issue (bug_risk): parse_value returns a str, but BigInt is now declared as NewType("BigInt", int), so GraphQL inputs annotated as BigInt are delivered to resolvers as strings despite their type contract. Resolver code performing numeric operations receives string values and can fail or produce incorrect results.
Triggers: When a schema uses BigInt as an input or argument type and a client supplies a value.
Suggested fix: Return int(v) from parse_value so the runtime value matches the BigInt/int annotation.
| parse_value=lambda v: str(v), | |
| parse_value=lambda v: int(v), |

Description
Fixes the
DeprecationWarningraised on importingstrawberry_sqlalchemy_mapper, and brings thesupported
strawberry-graphqland Python versions up to date.strawberry-graphql0.288.0 deprecated passing a type tostrawberry.scalar(), which is howBigIntwas built — so merely importing this package emitted a warning.BigIntis now a plainNewType, with its scalar definition exposed asstrawberry_sqlalchemy_scalar_mapforregistration on the schema.
Adopting the replacement API pulled in a few unavoidable consequences, which is why this PR is
larger than the original issue suggests:
scalar_maponly exists instrawberry-graphql>= 0.288.0, so the floor had to move.strawberry-graphqlsurfaced two pre-existing incompatibilities inrelay.pyand in the relay tests.The commits are split by context and can be reviewed independently:
81d00546ed13baBigInttoStrawberryConfig.scalar_mapb55867bstrawberry-graphqlb4dbf39d07d097RELEASE.mdBreaking changes
BigIntegercolumns require registering the scalar map. Nothing else in the library needschanging, but a schema with
BigIntegercolumns will not build without it:Minimum
strawberry-graphqlis now 0.288.0.Python 3.8 and 3.9 are no longer supported.
Relay node ids are exposed as
IDinstead ofGlobalID, following therelay_use_legacy_global_iddefault upstream. Client queries must use$id: ID!, or setStrawberryConfig(relay_use_legacy_global_id=True)to keep the old name.RELEASE.mdalso documents a fifth, upstream-caused change: combiningfirstwithbeforeon aconnection returns a different page since
strawberry-graphql0.322.2 corrected the
slicing. Our
test_query_with_last_and_beforewas passingfirstdespite its name, and relied onthe old behaviour; it now uses
last, which is what it always intended to cover.Also fixed along the way
typing-extensionswas an undeclared dependency. It is imported at runtime infield.pyand
mapper.py, but only resolved becausestrawberry-graphqlhappens to require it. Nowdeclared explicitly.
scalars.pyhad no test coverage at all;tests/test_scalars.pyis the first.Testing
Run against PostgreSQL in containers, not just locally:
mypyandruff check/ruff format— cleanWindows was not exercised locally (
SUPPORTED_DBSis empty there, so the database tests skip).Types of Changes
Issues Fixed or Closed by This PR
strawberry-graphqlwell past the 0.255.0 that PR proposednox,nox-poetry,pytest,pytest-asyncio,pytest-covandruffare updated here (to newer versions than it proposes).sqlalchemy,greenlet,packagingandsetuptoolsare not touched by this PR and still want bumping.Checklist
Summary by Sourcery
Modernize scalar registration and Strawberry GraphQL compatibility while raising the supported Python baseline to 3.10.
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: