fix(openhexaClient): circular dependency#277
Conversation
nazarfil
force-pushed
the
fix/circular-dependency
branch
from
f07d9ec to
ee48b33
Compare
yolanfery
left a comment
yolanfery
left a comment
There was a problem hiding this comment.
Thanks for taking care, I had not seen there was an issue
Let me know if you need help fixing it, happy to take it over since I likely caused it
| def _detect_graphql_breaking_changes_if_needed(token): | ||
| """Detect breaking changes if not done recently between the schema referenced in the SDK and the server using graphql-core.""" | ||
| ONE_HOUR = 60 * 60 | ||
| now_timestamp = int(datetime.now().timestamp()) | ||
| if not settings.last_breaking_change_check or now_timestamp - settings.last_breaking_change_check > ONE_HOUR: | ||
| _detect_graphql_breaking_changes(token) | ||
| settings.last_breaking_change_check = now_timestamp | ||
|
|
||
|
|
||
| def _detect_graphql_breaking_changes(token): | ||
| """Detect breaking changes between the schema referenced in the SDK and the server using graphql-core.""" | ||
| stored_schema_obj = build_schema((Path(__file__).parent / "graphql" / "schema.generated.graphql").open().read()) | ||
| server_schema_obj = build_client_schema( | ||
| _query_graphql(get_introspection_query(input_value_deprecation=True), token=token) | ||
| ) | ||
|
|
||
| breaking_changes = find_breaking_changes(stored_schema_obj, server_schema_obj) | ||
| if breaking_changes: | ||
| current_version, latest_version = get_library_versions() | ||
| click.secho( | ||
| f"⚠️ Breaking changes detected between the SDK (version {current_version}) and the server:", | ||
| fg="red", | ||
| ) | ||
| for change in breaking_changes: | ||
| click.secho(f"- {change.description}", fg="yellow") | ||
| click.secho( | ||
| "This could lead to unexpected results.\n" | ||
| f"Please update the SDK to the latest version {latest_version} " | ||
| f"(using `pip install openhexa-sdk=={latest_version}`) or use a version of the SDK compatible with the server.", | ||
| fg="red", | ||
| ) | ||
|
|
||
|
|
||
| def graphql(query: str, variables=None, token=None): | ||
| """Check that there is no breaking change and perform a GraphQL request.""" | ||
| _detect_graphql_breaking_changes_if_needed(token) | ||
| return _query_graphql(query, variables, token) | ||
|
|
||
|
|
||
| def _query_graphql(query: str, variables=None, token=None): | ||
| """Perform a GraphQL request.""" | ||
| url = settings.api_url + "/graphql/" | ||
| if token is None: | ||
| token = settings.access_token | ||
|
|
||
| if token is None: | ||
| raise InvalidTokenError("No token found for workspace") | ||
|
|
||
| if settings.debug: | ||
| click.echo("") | ||
| click.echo("Graphql Query:") | ||
| click.echo(f"URL: {url}") | ||
| click.echo(f"Query: {query}") | ||
| click.echo(f"Variables: {variables}") | ||
|
|
||
| session = create_requests_session() | ||
|
|
||
| response = session.post( | ||
| url, | ||
| headers={ | ||
| "User-Agent": f"openhexa-cli/{version('openhexa.sdk')}", | ||
| "Authorization": f"Bearer {token}", | ||
| }, | ||
| json={"query": query, "variables": variables}, | ||
| ) | ||
| try: | ||
| response.raise_for_status() | ||
| except requests.exceptions.HTTPError as e: | ||
| raise GraphQLError(str(e)) | ||
|
|
||
| data = response.json() | ||
|
|
||
| if settings.debug: | ||
| click.echo("Graphql Response:") | ||
| click.echo(data) | ||
| click.echo("") | ||
|
|
||
| if data.get("errors"): | ||
| if data.get("errors")[0].get("extensions", {}).get("code") == "UNAUTHENTICATED": | ||
| raise InvalidTokenError | ||
| raise GraphQLError(data["errors"]) | ||
| return data["data"] | ||
|
|
||
|
|
||
| def get_library_versions() -> tuple[str, str]: | ||
| """Return the current version and the one on PyPi.""" | ||
| # Get the currently installed version | ||
| installed_version = version("openhexa.sdk") | ||
|
|
||
| # Get the latest version available on PyPI | ||
| try: | ||
| response = requests.get("https://pypi.org/pypi/openhexa.sdk/json") | ||
| latest_version = response.json()["info"]["version"] | ||
| return installed_version, latest_version | ||
| except requests.RequestException: | ||
| logging.error( | ||
| "Could not check for the latest version of the openhexa.sdk package.", | ||
| exc_info=True, | ||
| ) | ||
| return installed_version, installed_version |
There was a problem hiding this comment.
I would keep those in the original place if possible
There was a problem hiding this comment.
I tried moving it back into cli, but ended again with circular dependency when importing it to sdk
| @@ -8,24 +8,19 @@ | |||
| import os | |||
There was a problem hiding this comment.
Did you understand what is causing the circular import ? I have trouble understanding it , thanks
There was a problem hiding this comment.
Yeah, it was the logger import from sdk :D , i could move now everything back to original place in CLI
from openhexa.sdk.pipelines.log_level import LogLevel
that pulls in
sdk.pipelines
which imports sdk.workspaces
which (eventually) imports openhexa.graphql.openhexa_client
which was importing cli.settings
There was a problem hiding this comment.
GraphQL client was defined in .cli , but was using import form .sdk, so when i tried using client in .sdk it created ciruclar imports. I have now moved grpahql client outside sdk and cli, however as it is coupled with .cli.settings, it can stay in .cli . we need to be careful not importing .sdk packages into it
nazarfil
force-pushed
the
fix/circular-dependency
branch
2 times, most recently
from
7a2e9b9 to
1c43637
Compare
nazarfil
force-pushed
the
fix/circular-dependency
branch
from
1c43637 to
37ea1cc
Compare
|
What about #278 ? I have a few issues with your PR :
|
We can go with the proposed workaround to unblock users and then change the implementation to initialise constructor for CLI and SDK separately |
this still needs some work:
|
Here is a PR covering those TODOs, thanks for discussions around that contributing to improve the current setup |
2 participants
I have refactored OpenHexa client from api to avoid circular dependencies.
https://github.com/BLSQ/openhexa-sdk-python/actions/runs/15774267318/job/44465049476