-
Notifications
You must be signed in to change notification settings - Fork 89
Fix object variables #266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix object variables #266
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # Instructions to AI Agents | ||
|
|
||
| ## Dev environment | ||
|
|
||
| * The project manages dependencies, virtual environment and installation using `poetry`. Always use `poetry run` to call commands in the virtual environment. | ||
| * This Python 3 project uses PEP8 and best practices. | ||
| * The project uses [pre-commit](https://pre-commit.com) to verify `flake8`, `black`, check examples and run tests. | ||
| * Format code using `black` and rules defined in `pyproject.toml`, it's executed from `pre-commit` environment/sandbox. | ||
| * Quality check and lint is done using `flake8` and rules defined in `.flake8`, it's executed from `pre-commit` environment/sandbox. | ||
|
|
||
| ## Code Guidelines | ||
| * Follow `The Zen of Python, by Tim Peters` (`import this`). | ||
| * Do not use useless comments. The code must be legible, thus comments should be mostly useless. | ||
| * Use docstrings as a way to document in a testable way. We do not want broken docs. | ||
| * Do not leave trailing whitespaces. | ||
| * Review every added statement: Is it needed? Is it useful? Is it correct? | ||
| * Do not leave useless statements (including imports). | ||
| * Use `black` to format. | ||
| * Use `flake8` to verify quality and rules. | ||
| * Always produce code similar to the sibling statements and functions. Review if same patterns are being followed. Avoid bringing in new patterns. | ||
| * Unless explicitly asked, prefer top level imports. | ||
| * Unless explicitly asked, prefer early return with the smallest branch (number of lines of code) | ||
| * Use reStructuredText documentation. | ||
| * See project instructions at `./README.rst` | ||
|
|
||
| ## Testing instructions | ||
| * The project uses `pytest` to run unit tests and doctests. | ||
| * Unit tests are stored in `tests/` directory and all have the `test-` prefix followed by a kebab-case descriptive name. | ||
| * Doctests are preferred over unit-tests, use it whenever possible. Just use test files when mock and other setup is needed. | ||
| * If you receive errors about types being already registered in the schema `Schema already has XXX`, create new `Schema()` in each test and declare `__schema__ = newly_created_schema` for types in that test. | ||
| * Test coverage must be 100%, respecting configuration stored at `.coveragerc`. | ||
|
|
||
| ## Examples instructions | ||
| * Meaningful examples stored in `examples/` directory. | ||
| * Examples must always be checked as an additional test, they must always run. | ||
| * Some examples requires environment variables such as `GH_TOKEN`, `SHOP_STORE` and `SHOP_TOKEN` or arguments such as `--token` to provide per-user, sensitive/private information to run. Never disclose these. | ||
| * It's possible to re-run `./update-schema.sh` using `NO_DOWNLOAD=1` environment variables to avoid the requirement of such private tokens. However these will not refresh to the latest API. | ||
| * When `sgqlc` is changed, always re-run `./update-schema.sh` and `./update-operations.sh`. | ||
|
|
||
| ## PR Instructions | ||
|
|
||
| * Always run `pre-commit` before creating a commit or push. | ||
| * Avoid patch-noise (useless changes), review line by line: "this really needs to be changed?" | ||
|
|
||
| * Commits should be atomic: each commit must pass `pre-commit` on its own. | ||
| * Commit messages should be brief and meaningful, describing exactly what was done. | ||
| * The first line of the commit should be terse and under 72 chars. | ||
| * If the commit fixes a bug, add the trailing line: `Closes: #XXX`, where `XXX` is the GitHub issue number. | ||
| * Do **NOT** open Pull Request without prior **HUMAN REVIEW**, they will be immediately closed. | ||
| * If the commit had AI assistance, write it in the commit message. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,7 @@ | |
| import urllib.parse | ||
| import urllib.request | ||
|
|
||
| from .base import BaseEndpoint, add_query_to_url | ||
| from .base import BaseEndpoint, JSONEncoder, add_query_to_url | ||
|
|
||
|
Comment on lines
+38
to
39
|
||
|
|
||
| class HTTPEndpoint(BaseEndpoint): | ||
|
|
@@ -210,12 +210,13 @@ def get_http_post_request(self, query, variables, operation_name, headers): | |
| 'query': query, | ||
| 'variables': variables, | ||
| 'operationName': operation_name, | ||
| } | ||
| }, | ||
| cls=JSONEncoder, | ||
| ).encode('utf-8') | ||
| headers.update( | ||
| { | ||
| 'Content-Type': 'application/json; charset=utf-8', | ||
| 'Content-Length': len(post_data), | ||
| 'Content-Length': str(len(post_data)), | ||
| } | ||
| ) | ||
| return urllib.request.Request( | ||
|
|
@@ -228,7 +229,7 @@ def get_http_get_request(self, query, variables, operation_name, headers): | |
| params['operationName'] = operation_name | ||
|
|
||
| if variables: | ||
| params['variables'] = json.dumps(variables) | ||
| params['variables'] = json.dumps(variables, cls=JSONEncoder) | ||
|
|
||
| url = add_query_to_url(self.url, params) | ||
| return urllib.request.Request(url=url, headers=headers, method='GET') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,7 @@ | |
| import json | ||
| import httpx | ||
|
|
||
| from .base import add_query_to_url | ||
| from .base import JSONEncoder, add_query_to_url | ||
| from .http import HTTPEndpoint | ||
| from typing import Optional, Union, Dict | ||
|
|
||
|
|
@@ -226,15 +226,25 @@ def _log_httpx_error(self, query, request, exc): | |
|
|
||
| def get_http_post_request(self, query, variables, operation_name, headers): | ||
| '''Create an HTTP POST request for the query.''' | ||
| return self.client.build_request( | ||
| method='POST', | ||
| url=self.url, | ||
| headers=headers, | ||
| json={ | ||
| post_data = json.dumps( | ||
| { | ||
| 'query': query, | ||
| 'variables': variables, | ||
| 'operationName': operation_name, | ||
|
Comment on lines
+229
to
233
|
||
| }, | ||
| cls=JSONEncoder, | ||
| ).encode('utf-8') | ||
| headers.update( | ||
| { | ||
| 'Content-Type': 'application/json; charset=utf-8', | ||
| 'Content-Length': str(len(post_data)), | ||
| } | ||
| ) | ||
| return self.client.build_request( | ||
| method='POST', | ||
| url=self.url, | ||
| headers=headers, | ||
| content=post_data, | ||
| ) | ||
|
|
||
| def get_http_get_request(self, query, variables, operation_name, headers): | ||
|
|
@@ -244,7 +254,7 @@ def get_http_get_request(self, query, variables, operation_name, headers): | |
| params['operationName'] = operation_name | ||
|
|
||
| if variables: | ||
| params['variables'] = json.dumps(variables) | ||
| params['variables'] = json.dumps(variables, cls=JSONEncoder) | ||
|
|
||
| url = add_query_to_url(self.url, params) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,7 +40,7 @@ | |
| import requests | ||
|
|
||
|
|
||
| from .base import BaseEndpoint, add_query_to_url | ||
| from .base import BaseEndpoint, JSONEncoder, add_query_to_url | ||
|
|
||
|
Comment on lines
+43
to
44
|
||
|
|
||
| class RequestsEndpoint(BaseEndpoint): | ||
|
|
@@ -239,9 +239,15 @@ def get_http_post_request(self, query, variables, operation_name, headers): | |
| 'query': query, | ||
| 'variables': variables, | ||
| 'operationName': operation_name, | ||
| } | ||
| }, | ||
| cls=JSONEncoder, | ||
| ).encode('utf-8') | ||
| headers.update({'Content-Type': 'application/json; charset=utf-8'}) | ||
| headers.update( | ||
| { | ||
| 'Content-Type': 'application/json; charset=utf-8', | ||
| 'Content-Length': str(len(post_data)), | ||
| } | ||
| ) | ||
| return requests.Request( | ||
| url=self.url, | ||
| auth=self.auth, | ||
|
|
@@ -256,7 +262,7 @@ def get_http_get_request(self, query, variables, operation_name, headers): | |
| params['operationName'] = operation_name | ||
|
|
||
| if variables: | ||
| params['variables'] = json.dumps(variables) | ||
| params['variables'] = json.dumps(variables, cls=JSONEncoder) | ||
|
|
||
| url = add_query_to_url(self.url, params) | ||
| return requests.Request( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,13 @@ | ||
| import gzip | ||
| import io | ||
| import json | ||
| import pytest | ||
| import urllib.error | ||
| import urllib.request | ||
|
|
||
| from unittest.mock import MagicMock, patch | ||
| from sgqlc.endpoint.http import HTTPEndpoint, add_query_to_url | ||
| from sgqlc.types import Schema, Type | ||
| from sgqlc.types import Schema, Type, Input | ||
| from sgqlc.operation import Operation | ||
|
|
||
| test_url = 'http://some-server.com/graphql' | ||
|
|
@@ -160,7 +161,18 @@ def check_request_variables(req, variables): | |
| query = get_request_url_query(req) | ||
| received = json.loads(query.get('variables', 'null')) | ||
|
|
||
| assert received == variables | ||
| if not variables: | ||
| assert received == variables | ||
| return | ||
|
|
||
| # Convert Input types to their JSON representation for comparison | ||
| expected = {} | ||
| for k, v in variables.items(): | ||
| if hasattr(v, '__json_data__'): | ||
| expected[k] = v.__json_data__ | ||
|
Comment on lines
+168
to
+172
|
||
| else: | ||
| expected[k] = v | ||
| assert received == expected | ||
|
|
||
|
|
||
| def check_request_operation_name(req, operation_name): | ||
|
|
@@ -674,6 +686,56 @@ def test_server_http_error_list_message(mock_urlopen): | |
| check_mock_urlopen(mock_urlopen) | ||
|
|
||
|
|
||
| @patch('urllib.request.urlopen') | ||
| def test_variables_with_input_type(mock_urlopen): | ||
| 'Test if variables with sgqlc.types.Input are properly serialized' | ||
|
|
||
| schema = Schema() | ||
|
|
||
| # MyInput may be declared if doctests were processed by pytest | ||
| if 'MyInput' in schema: | ||
| schema -= schema.MyInput | ||
|
|
||
| class MyInput(Input): | ||
| __schema__ = schema | ||
| a_str = str | ||
| a_int = int | ||
|
|
||
| configure_mock_urlopen( | ||
| mock_urlopen, | ||
| graphql_response_ok, | ||
| graphql_headers_ok, | ||
| ) | ||
|
|
||
| variables = {'input': MyInput(a_str='hello', a_int=42)} | ||
|
|
||
| endpoint = HTTPEndpoint(test_url) | ||
| data = endpoint(graphql_query, variables) | ||
| assert data['data']['repository']['issues']['nodes'][0]['number'] == 1 | ||
| check_mock_urlopen(mock_urlopen, variables=variables) | ||
|
|
||
|
|
||
| @patch('urllib.request.urlopen') | ||
| def test_variables_with_bogus_type(mock_urlopen): | ||
| 'Test if variables with non-serializable types raise TypeError' | ||
|
|
||
| class BogusType: | ||
| pass | ||
|
|
||
| configure_mock_urlopen( | ||
| mock_urlopen, | ||
| graphql_response_ok, | ||
| graphql_headers_ok, | ||
| ) | ||
|
|
||
| variables = {'input': BogusType()} | ||
|
|
||
| endpoint = HTTPEndpoint(test_url) | ||
| with pytest.raises(TypeError): | ||
| endpoint(graphql_query, variables) | ||
| assert not mock_urlopen.called | ||
|
|
||
|
|
||
| # add_query_to_url(): | ||
| # test paths not already tested, here just the repeated query | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doctest example defines
MyInput(Input)without an explicit schema and with a very generic type name. Since the project runs--doctest-modules, this is likely to collide with existing doctests (e.g.sgqlc.typesalready definesMyInput) and can fail with “schema already has MyInput”, plus it pollutesglobal_schemafor subsequent tests. Use a unique input name and/or bind the example type to a freshSchema()via__schema__(and remove any existing type from that schema) so the doctest is isolated.