-
Notifications
You must be signed in to change notification settings - Fork 230
fix(gfql): preflight remote engine requests #2005
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
Open
lmeyerov
wants to merge
7
commits into
master
Choose a base branch
from
fix/gfql-1957-remote-engine-preflight
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
cca8884
fix(gfql): preflight remote engine requests
lmeyerov 8d362f2
ci(gfql): justify remote resolver input type
lmeyerov 268c93a
ci(gfql): keep remote contract tests py38-compatible
lmeyerov 8289bc2
ci(gfql): classify remote engine contract lane
lmeyerov 5defe0d
test(engine): recognize remote resolver wrapper
lmeyerov 8825e2b
refactor(remote): type engine preflight contracts
lmeyerov 55161a1
docs(changelog): complete remote engine preflight
lmeyerov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
127 changes: 127 additions & 0 deletions
127
graphistry/tests/compute/test_remote_engine_contract.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| """Contract tests for explicit engines on remote compute calls.""" | ||
|
|
||
| import typing | ||
| from typing import Optional | ||
| from unittest.mock import MagicMock, patch | ||
| from typing_extensions import Literal | ||
|
|
||
| import pandas as pd | ||
| import pytest | ||
|
|
||
| from graphistry.Engine import EngineAbstractType | ||
| from graphistry.Plottable import Plottable | ||
| from graphistry.compute.ast import ASTNode | ||
| from graphistry.compute.chain import Chain | ||
| from graphistry.compute.chain_remote import chain_remote_generic | ||
| from graphistry.compute.exceptions import ErrorCode, GFQLRemoteError | ||
| from graphistry.compute.python_remote import python_remote_generic | ||
| from graphistry.compute.remote_df_io import RemoteAPIName | ||
|
|
||
|
|
||
| TASK = "def task(g):\n return g\n" | ||
| QUERY = Chain([ASTNode(filter_dict={"type": "Person"})]) | ||
| _PostTarget = Literal[ | ||
| "graphistry.compute.chain_remote.requests.post", | ||
| "graphistry.compute.python_remote.requests.post", | ||
| ] | ||
|
|
||
|
|
||
|
|
||
| class Posted(Exception): | ||
| """Stop a test after the request reaches the mocked transport.""" | ||
|
|
||
|
|
||
| def mock_plottable(dataset_id: Optional[str] = None) -> MagicMock: | ||
| """Build the minimum graph state used by both remote entry points.""" | ||
| graph = MagicMock() | ||
| graph._dataset_id = dataset_id | ||
| graph._edges = pd.DataFrame({"s": [0], "d": [1]}) | ||
| graph._nodes = pd.DataFrame({"id": [0, 1]}) | ||
| graph._privacy = None | ||
| graph._url_params = {} | ||
| graph.session.api_token = "refreshed-token" | ||
| graph.session.certificate_validation = True | ||
| graph.base_url_server.return_value = "https://test.graphistry.com" | ||
|
|
||
| def upload(*, validate: bool) -> MagicMock: | ||
| graph._dataset_id = "uploaded-dataset" | ||
| return graph | ||
|
|
||
| graph.upload.side_effect = upload | ||
| return graph | ||
|
|
||
|
|
||
| def call_remote( | ||
| api_name: RemoteAPIName, | ||
| graph: Plottable, | ||
| engine: EngineAbstractType, | ||
| *, | ||
| with_creds: bool, | ||
| ) -> typing.NoReturn: | ||
| """Call one remote entry point with matching mock credentials.""" | ||
| api_token = "token" if with_creds else None | ||
| dataset_id = "dataset" if with_creds else None | ||
| if api_name == "gfql_remote": | ||
| chain_remote_generic( | ||
| graph, | ||
| QUERY, | ||
| api_token=api_token, | ||
| dataset_id=dataset_id, | ||
| engine=engine, | ||
| format="json", | ||
| validate=False, | ||
| ) | ||
| else: | ||
| python_remote_generic( | ||
| graph, | ||
| TASK, | ||
| api_token=api_token, | ||
| dataset_id=dataset_id, | ||
| engine=engine, | ||
| format="json", | ||
| output_type="json", | ||
| validate=False, | ||
| ) | ||
| raise AssertionError("remote call returned before transport") | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("api_name", "post_target"), | ||
| [ | ||
| ("gfql_remote", "graphistry.compute.chain_remote.requests.post"), | ||
| ("python_remote", "graphistry.compute.python_remote.requests.post"), | ||
| ], | ||
| ) | ||
| @pytest.mark.parametrize("engine", ["pandas", "cudf"]) | ||
| def test_explicit_supported_engine_is_sent_unchanged( | ||
| api_name: RemoteAPIName, post_target: _PostTarget, engine: EngineAbstractType | ||
| ) -> None: | ||
| graph = mock_plottable("dataset") | ||
| with patch(post_target, side_effect=Posted) as post: | ||
| with pytest.raises(Posted): | ||
| call_remote(api_name, graph, engine, with_creds=True) | ||
| assert post.call_args.kwargs["json"]["engine"] == engine | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("api_name", "post_target"), | ||
| [ | ||
| ("gfql_remote", "graphistry.compute.chain_remote.requests.post"), | ||
| ("python_remote", "graphistry.compute.python_remote.requests.post"), | ||
| ], | ||
| ) | ||
| @pytest.mark.parametrize("engine", ["polars", "polars-gpu"]) | ||
| def test_explicit_unsupported_engine_declines_before_side_effects( | ||
| api_name: RemoteAPIName, post_target: _PostTarget, engine: EngineAbstractType | ||
| ) -> None: | ||
| graph = mock_plottable() | ||
| with patch(post_target) as post: | ||
| with pytest.raises(GFQLRemoteError) as excinfo: | ||
| call_remote(api_name, graph, engine, with_creds=False) | ||
|
|
||
| assert excinfo.value.code == ErrorCode.E405 | ||
| assert excinfo.value.context["field"] == "engine" | ||
| assert excinfo.value.context["value"] == engine | ||
| graph._pygraphistry.refresh.assert_not_called() | ||
| graph.upload.assert_not_called() | ||
| post.assert_not_called() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.