Skip to content

0.3.3.dev9 - add support for optional arguments #36

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.

Sign up for GitHub

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

Merged
eupharis merged 1 commit into from
Apr 14, 2025
Merged

0.3.3.dev9 - add support for optional arguments #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion LICENSE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
MIT License

Copyright (c) 2023 PolyAPI Inc.
Copyright (c) 2025 PolyAPI Inc.

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down
7 changes: 6 additions & 1 deletion polyapi/execute.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from typing import Dict
import requests
from requests import Response
from polyapi.config import get_api_key_and_url
Expand All @@ -7,10 +8,14 @@
def execute(function_type, function_id, data) -> Response:
""" execute a specific function id/type
"""
data_without_None = data
if isinstance(data, Dict):
data_without_None = {k: v for k, v in data.items() if v is not None}

Comment on lines +11 to +14
Copy link
Member

@aarongoin aarongoin Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that users can't explicitly pass None as an argument though?

Copy link
Member

@aarongoin aarongoin Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the gateway is ultimately responsible for handling the required and removeIfNotPresentOnExecute argument properties. At least today.

Copy link
Member Author

@eupharis eupharis Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. You can't explicitly pass None now.

Can we have the gateway know if it's Python versus JS? And if it's Python treat None as undefined?

There's no such thing as undefined in Python.

Alternatively we could make it a general feature of the gateway that both None and undefined are stripped out. That would make things more predictable cross-language.

Copy link
Member Author

@eupharis eupharis Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aarongoin alternatively I can write my own Undefined type. And then strip it out client side. Or pass it to the server as a special value.

But it's going to be much more work to add unions everywhere with the custom types.

And the more Pythonic approach in general is to use Nones for this:

https://stackoverflow.com/a/23828324

Copy link
Member Author

@eupharis eupharis Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aarongoin wait a minute. you can't POST undefined over the wire as JSON. It's not JSON. How is the JS client passing this right now? the string 'undefined'?

Copy link
Member

@aarongoin aarongoin Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we should treat null == undefined == None. Undefined is a special JavaScript type that should probably stay in JS only. And then in the gateway we throw an error if required if value is null/undefined, and if removeIfNotPresentOnExecute is true then we also strip the argument entirely.

Copy link
Member Author

@eupharis eupharis Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aarongoin love it!

api_key, api_url = get_api_key_and_url()
headers = {"Authorization": f"Bearer {api_key}"}
url = f"{api_url}/functions/{function_type}/{function_id}/execute"
resp = requests.post(url, json=data, headers=headers)
resp = requests.post(url, json=data_without_None, headers=headers)
# print(resp.status_code)
# print(resp.headers["content-type"])
if resp.status_code < 200 or resp.status_code >= 300:
Expand Down
2 changes: 1 addition & 1 deletion polyapi/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def _get_type_schema(json_type: str, python_type: str, schemas: List[Dict]):
return schema


def _get_type(expr: ast.expr | None, schemas: List[Dict]) -> Tuple[str, Dict | None]:
def _get_type(expr: ast.expr | None, schemas: List[Dict]) -> Tuple[Any, Any, Any]:
Copy link
Member Author

@eupharis eupharis Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a mypy fix

if not expr:
return "any", "Any", None
python_type = get_python_type_from_ast(expr)
Expand Down
3 changes: 3 additions & 0 deletions polyapi/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ def parse_arguments(
arg_string += (
f" {a['name']}: {add_type_import_path(function_name, arg_type)}"
)
if not a["required"]:
arg_string += " = None"

description = a.get("description", "")
description = description.replace("\n", " ")
if description:
Expand Down
3 changes: 2 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ requires = ["setuptools>=61.2", "wheel"]

[project]
name = "polyapi-python"
version = "0.3.3.dev8"
version = "0.3.3.dev9"
description = "The Python Client for PolyAPI, the IPaaS by Developers for Developers"
authors = [{ name = "Dan Fellin", email = "dan@polyapi.io" }]
dependencies = [
Expand Down Expand Up @@ -33,3 +33,4 @@ exclude = ["polyapi/poly*", "polyapi/vari*", "polyapi/.config.env", "polyapi/cac
[tool.mypy]
# for now redef errors happen sometimes, we will clean this up in the future!
disable_error_code = "no-redef,name-defined"
implicit_optional = true