-
Notifications
You must be signed in to change notification settings - Fork 1
4523 fix python client to save generate command arguments and reuse them #53
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
4523 fix python client to save generate command arguments and reuse them #53
Conversation
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.
Heads up, when you make a PR you need to assign someone to review it. Right now it looks like no one is actually assigned so no one will get notified. (Though me reviewing appears to have assigned me as the reviewer. 🙂)
Now wrt to this PR: Conceptually I like where you're trying to go, but I think we're being a little too clever with the cached values and it's going to be frustrating for end users. So let's simplify the logic here.
Part of the issue with using this is_less_restrictive
utility is that I will never be able to generate MORE restrictively which is going to be really confusing for people that don't know about our cached arguments. While you and I as internal devs know about and might even manually adjust the .config.env file, we can't expect our users to know about this or manually update it.
So here's what I'm thinking:
When a user runs the generate command explicitly we overwrite all the cached values with the values (or lack of values) passed in from the command line. So if I execute the generate command and I only put in function ids then all the other cached values should get blown away and the cache should only remember my function ids. Or if I run the generate command with NO arguments then all the cached values get blown away.
When WE run generate programmatically from inside the CLI--like after deploying a function--we pick up the cached values and reuse them.
Does that make sense? Sorry if I wasn't clear enough in my ticket!
Wrote it to only have the cache used in function uploading and initial - these are the only ones I could find that used generate without explicit call. |
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.
Nice. Getting pretty close. Just a few more comments.
polyapi/config.py
Outdated
config.remove_option("polyapi", "last_generate_contexts_used") if config.has_option("polyapi", "last_generate_contexts_used") else None |
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.
Slightly more explicit behavior written this way:
else: | |
config.remove_option("polyapi", "last_generate_contexts_used") if config.has_option("polyapi", "last_generate_contexts_used") else None | |
elif config.has_option("polyapi", "last_generate_contexts_used"): | |
config.remove_option("polyapi", "last_generate_contexts_used") |
polyapi/config.py
Outdated
config.remove_option("polyapi", "last_generate_names_used") if config.has_option("polyapi", "last_generate_names_used") else None |
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.
else: | |
config.remove_option("polyapi", "last_generate_names_used") if config.has_option("polyapi", "last_generate_names_used") else None | |
elif config.has_option("polyapi", "last_generate_names_used"): | |
config.remove_option("polyapi", "last_generate_names_used") |
polyapi/config.py
Outdated
config.remove_option("polyapi", "last_generate_function_ids_used") if config.has_option("polyapi", "last_generate_function_ids_used") else None |
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.
else: | |
config.remove_option("polyapi", "last_generate_function_ids_used") if config.has_option("polyapi", "last_generate_function_ids_used") else None | |
elif config.has_option("polyapi", "last_generate_function_ids_used"): | |
config.remove_option("polyapi", "last_generate_function_ids_used") |
|
||
if names: | ||
params["names"] = names | ||
|
||
if function_ids: | ||
params["functionIds"] = function_ids |
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.
👍
polyapi/generate.py
Outdated
generate_msg = f"Generating Poly Python SDK for contexts ${contexts}..." if contexts else "Generating Poly Python SDK..." | ||
print(generate_msg, end="", flush=True) |
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.
Looks like we dropped this message? Let's add back for the user and update so it's a little smarter given all our generate arguments.
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.
Okay, I think this is the last part we need to resolve and then this PR should be ready.
generate_parser.add_argument("--names", type=str, required=False, help="Resource names to generate (comma-separated)") | ||
generate_parser.add_argument("--function-ids", type=str, required=False, help="Function IDs to generate (comma-separated)") |
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.
Nice catch. 👍
Oh, and one important thing to update when making changes to any of our clients: update the package version! 😃 For python we currently use a |
…ommand-arguments-and-reuse-them
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.
Nice job, Ashir! Send it! 🚀
* # Feature (2970): Update python client to support setup command (#22) * # Feature (2970): Update python client to support setup command - Function add command now support --execution-api-key - Extra Old Function call removed * improve polyapi-python setup (#24) * improve polyapi-python setup * # Feature (3019): improve polyapi-python setup (#25) * # Feature (3019): improve polyapi-python setup * # Feature (3019): improve polyapi-python setup - UUID Validation check added --------- Co-authored-by: Sudipta at TechJays <sudipta.kumar@techjays.com> * # Feature (3007): Update python -m polyapi function add --logs options (#23) * # Feature (3007): Update python -m polyapi function add --logs options - if --logs added, then value must enabled or disabled - If Nothing passed the value is default disabled - pyproject.toml version updated * Project Glide + Refactor main command line args parsing (#26) * Refactor main command line args parsing, adding prepare and sync commands to enable project glide workflows for python * improved tests * updating version * fix for poly cache directory path construction * one more adjustment to the deployables cache directory so there can't be any conflict with any custom namespace * this better? * verbose logging on upload code to see what's failing in CI/CD * bumpity * whoops * so close * better? * okay this should be the fix * is it this? * maybe * oh for the love of pete * whatever. might be a pypi issue * removing verbose logging * fixing bugs in sync command to use correct api urls * update logging * lint * improved auth * last fix for function sync * fix bug when comment arguments don't align with the function * try forcing the poly directory to exist * test logging * remove debug logging * fixing project glide deployable types and bumping the version * fixing missing arguments in python client function upload * fixing return type for trained functions * fix bug preventing use of poly sync command locally * next version of client! * EN #3183 allow null logs flag for python client (#28) * let the typing_extensions versions increase to support latest openai pypi package version * update dependency in one more place * Some bug fixes for python client (#29) * fixed bug with parsing python functions without any types, and bug where functions with multiple deployment receipts were getting mangled * whoops. uncommenting tests * last test fix * 0.3.2 * add poly schemas support (#31) * onward * adding schemas for Pythonland! * onward * next * next * next * next * test * next * next * next * little tweak for A-Aron * fix * next * update to v4 * v4 everywhere * bump version * add new version * remove warning, just go with any type for now * better generate printed messages, fix generate bug after function add * Update python version (#32) * Update python image (#33) * Update python version * Updated version * Rollback version * onward (#34) * woot! we have some better return types * toward working tests - except parser/deployables * more * getting there * next * onawrd * next * next * next * next * next (#35) * improve intellisense detection of schemas * release 0.3.3.dev8, fix misleading generate after setup * 0.3.3.dev9 - add support for optional arguments (#36) * next * release 0.3.3.dev10 * EN #3943 update to support SFX serverSideAsync True by setting correct return type (#39) * deploying version 0.3.3 for R22 * upgrade version * 4084 - revert strippping none values from function arguments during execution * P2) Update clients and specs endpoint so when generating with no-types argument all schemas get excluded (#38) * added no type option * version updated * 4010 generate contexts (#43) * make contexts truly optional * P3) (Optoro) Allow variable to be secret in the UI, but gettable in functions, and prevent secret variables from being made non-secret (#42) * secret -> secrecy - updated python client * comment fixed * add generate contexts (#45) * adds mtls and direct execute options (#44) * adds mtls and direct execute support * support for direct execute from client * fixed mtls * removed unused dep * polyCustom - prevent rewrites of executionId (#46) * 4292 (#47) * create mock schemas to fit everything when using no types flag (#48) * EN #4348 flatten new-lines in arg descriptions (#50) * EN #4348 flatten new-lines in arg descriptions * EN #4348 bump version to 0.3.7.dev4 * EN #4360 fix return types for TS funcs (#49) * adding ability for python client server and client functions to add custom headers (#51) * fix type error! * try simple upgrade * changed version * 0.3.8.dev0 make it clearer that jsonschema parsing issue is warning not error * 4418 p2 bug on glide pre commit hook poly prepare make sure we git add any docstrings added from poly prepare (#55) * Fixed windows no deployables found bug * removed superfluous print * Fixed deployables not being staged properly * Added .venv to excluded directories * Bumped version up to 0.3.8.dev1 * 4523 fix python client to save generate command arguments and reuse them (#53) * Add caching for all arguments, add names and function-ids arguments * Fix restrictiveness logic to work on all arguments * Only use cache when indirectly generated * initialize cache with generate * initialize cache to generate * Update toml and config * Restore Generating... print message --------- Co-authored-by: Ashir Rao <ar2558@cornell.edu> * Update pydantic version to work with Python 3.13 (#54) * fix vari under no types (#56) * better import error (#59) * 4645 Add github action for polyapi-python unittests, fix polyapi-python unittests (#57) * make tests pass and github actions * comment + push * add dev_requirements * use dev requirements * using mkdir to avoid poly not existing * Revert deployables anf change whitespace for passing tests * undo diff * undo deployables.py change * Windows glide bug (#61) * Fixed windows find deployable command and fixed ai description generation urls * Changed version number * version command in python (#58) * version command in python * P2) Webhook Payload Type Blows Up our Python Client (#62) * adds a fail safe when generating resources * version increase * version increment * Fixing bug where users couldn't put a description in their polyConfig… (#60) * Fixing bug where users couldn't put a description in their polyConfig field for glide functions * removing test_bash which should not have been commited, and bumping the version * next * fix schema generation (#64) * remove bad ci file * Upgrading version to 0.3.8 (#67) --------- Co-authored-by: Sudipta at TechJays <sudipta.kumar@techjays.com> Co-authored-by: Aaron Goin <aarongoin@users.noreply.github.com> Co-authored-by: Eric Neumann <eneumann@users.noreply.github.com> Co-authored-by: Don Chiniquy <drchiniquy@gmail.com> Co-authored-by: nahuel-polyapi <nahuel@polyapi.io> Co-authored-by: Bboydozzy96 <federico.marchini@hotovo.com> Co-authored-by: FedeMarchiniHotovo <130762770+FedeMarchiniHotovo@users.noreply.github.com> Co-authored-by: Shina Akinboboye <shina@polyapi.io> Co-authored-by: Richard <github.agonize754@passmail.net> Co-authored-by: Shina Akinboboye <60622084+akinboboye@users.noreply.github.com> Co-authored-by: Daniel-Estoll <115661842+Daniel-Estoll@users.noreply.github.com> Co-authored-by: Ashir Rao <69091220+Ash1R@users.noreply.github.com> Co-authored-by: Ashir Rao <ar2558@cornell.edu>
* # Feature (2970): Update python client to support setup command (#22) * # Feature (2970): Update python client to support setup command - Function add command now support --execution-api-key - Extra Old Function call removed * improve polyapi-python setup (#24) * improve polyapi-python setup * # Feature (3019): improve polyapi-python setup (#25) * # Feature (3019): improve polyapi-python setup * # Feature (3019): improve polyapi-python setup - UUID Validation check added --------- Co-authored-by: Sudipta at TechJays <sudipta.kumar@techjays.com> * # Feature (3007): Update python -m polyapi function add --logs options (#23) * # Feature (3007): Update python -m polyapi function add --logs options - if --logs added, then value must enabled or disabled - If Nothing passed the value is default disabled - pyproject.toml version updated * Project Glide + Refactor main command line args parsing (#26) * Refactor main command line args parsing, adding prepare and sync commands to enable project glide workflows for python * improved tests * updating version * fix for poly cache directory path construction * one more adjustment to the deployables cache directory so there can't be any conflict with any custom namespace * this better? * verbose logging on upload code to see what's failing in CI/CD * bumpity * whoops * so close * better? * okay this should be the fix * is it this? * maybe * oh for the love of pete * whatever. might be a pypi issue * removing verbose logging * fixing bugs in sync command to use correct api urls * update logging * lint * improved auth * last fix for function sync * fix bug when comment arguments don't align with the function * try forcing the poly directory to exist * test logging * remove debug logging * fixing project glide deployable types and bumping the version * fixing missing arguments in python client function upload * fixing return type for trained functions * fix bug preventing use of poly sync command locally * next version of client! * EN #3183 allow null logs flag for python client (#28) * let the typing_extensions versions increase to support latest openai pypi package version * update dependency in one more place * Some bug fixes for python client (#29) * fixed bug with parsing python functions without any types, and bug where functions with multiple deployment receipts were getting mangled * whoops. uncommenting tests * last test fix * 0.3.2 * add poly schemas support (#31) * onward * adding schemas for Pythonland! * onward * next * next * next * next * test * next * next * next * little tweak for A-Aron * fix * next * update to v4 * v4 everywhere * bump version * add new version * remove warning, just go with any type for now * better generate printed messages, fix generate bug after function add * Update python version (#32) * Update python image (#33) * Update python version * Updated version * Rollback version * onward (#34) * woot! we have some better return types * toward working tests - except parser/deployables * more * getting there * next * onawrd * next * next * next * next * next (#35) * improve intellisense detection of schemas * release 0.3.3.dev8, fix misleading generate after setup * 0.3.3.dev9 - add support for optional arguments (#36) * next * release 0.3.3.dev10 * EN #3943 update to support SFX serverSideAsync True by setting correct return type (#39) * deploying version 0.3.3 for R22 * upgrade version * 4084 - revert strippping none values from function arguments during execution * P2) Update clients and specs endpoint so when generating with no-types argument all schemas get excluded (#38) * added no type option * version updated * 4010 generate contexts (#43) * make contexts truly optional * P3) (Optoro) Allow variable to be secret in the UI, but gettable in functions, and prevent secret variables from being made non-secret (#42) * secret -> secrecy - updated python client * comment fixed * add generate contexts (#45) * adds mtls and direct execute options (#44) * adds mtls and direct execute support * support for direct execute from client * fixed mtls * removed unused dep * polyCustom - prevent rewrites of executionId (#46) * 4292 (#47) * create mock schemas to fit everything when using no types flag (#48) * EN #4348 flatten new-lines in arg descriptions (#50) * EN #4348 flatten new-lines in arg descriptions * EN #4348 bump version to 0.3.7.dev4 * EN #4360 fix return types for TS funcs (#49) * adding ability for python client server and client functions to add custom headers (#51) * fix type error! * try simple upgrade * changed version * 0.3.8.dev0 make it clearer that jsonschema parsing issue is warning not error * 4418 p2 bug on glide pre commit hook poly prepare make sure we git add any docstrings added from poly prepare (#55) * Fixed windows no deployables found bug * removed superfluous print * Fixed deployables not being staged properly * Added .venv to excluded directories * Bumped version up to 0.3.8.dev1 * 4523 fix python client to save generate command arguments and reuse them (#53) * Add caching for all arguments, add names and function-ids arguments * Fix restrictiveness logic to work on all arguments * Only use cache when indirectly generated * initialize cache with generate * initialize cache to generate * Update toml and config * Restore Generating... print message --------- Co-authored-by: Ashir Rao <ar2558@cornell.edu> * Update pydantic version to work with Python 3.13 (#54) * fix vari under no types (#56) * better import error (#59) * 4645 Add github action for polyapi-python unittests, fix polyapi-python unittests (#57) * make tests pass and github actions * comment + push * add dev_requirements * use dev requirements * using mkdir to avoid poly not existing * Revert deployables anf change whitespace for passing tests * undo diff * undo deployables.py change * Windows glide bug (#61) * Fixed windows find deployable command and fixed ai description generation urls * Changed version number * version command in python (#58) * version command in python * P2) Webhook Payload Type Blows Up our Python Client (#62) * adds a fail safe when generating resources * version increase * version increment * Fixing bug where users couldn't put a description in their polyConfig… (#60) * Fixing bug where users couldn't put a description in their polyConfig field for glide functions * removing test_bash which should not have been commited, and bumping the version * next * fix schema generation (#64) * remove bad ci file * Upgrading version to 0.3.8 (#67) * 4655 p3 polyapi python schema errors lets fix (#63) * Changed encoding to utf-8 and added unit test * changed version * Updated version * Fixed find deployables command to ensure there are no duplicates (#65) * Fixed find deployables command to ensure there are no duplicates * Updated version * Updated version * Added check for LOGS_ENABLED env var and updated exceptions (#70) * Added check for LOGS_ENABLED env var and updated exceptions * Bumped version * allow higher stdlib_list * update in one more spot * increase version of truststore installed * added logger (#72) * added logger * bumped version * Change print to use logging (#73) * Monkey patched print to use logging module * Bumped version * EN #4845 fix function args schema bug for TypedDicts * Revert c612f6e (EN #4845 fix function args schema bug for TypedDicts) – accidental push * EN bump v to 0.3.9.dev8 * EN #4845 fix func arg schema bug with typed dicts (#75) * EN #4845 fix func arg schema bug with typed dicts * EN #4845 v0.3.9.dev9 * Tabi sdk (#74) * fixing client_id to be singe shared value per generation--matching typescript behavior * fixing some little type errors * tabi in the house! * tweaked to make table_id available on class, and adding description as a docstring comment for the class * bump version * oh lordy (#76) * One more missed f-string in tabi * Revert monkey patch (#77) * Revert "Monkey patched print to use logging module" This reverts commit a761c64. * bumped version * remove need for special logging process * fix GitHub action for polyapi python unittests, fix polyapi python unittests (#66) * make tests pass and github actions * comment + push * add dev_requirements * use dev requirements * using mkdir to avoid poly not existing * Revert deployables anf change whitespace for passing tests * undo diff * undo deployables.py change * new python-ci with pytest, correct actions syntax * add flask to the requirements, needed for a test * running into weird ord bug in python3.11, lets do simpler 7 char hash (#79) * running into weird ord bug in python3.11, lets do simpler 7 char hash * bump * fix tests * add workflow dispatch * try under 3.13 * 0.3.9.dev15: define some sort of scrub_keys * back up * actually fix tests * Revert "actually fix tests" This reverts commit 87caa04. * in sync with actions now? * update version for deploy --------- Co-authored-by: Sudipta at TechJays <sudipta.kumar@techjays.com> Co-authored-by: Dan Fellin <dan@polyapi.io> Co-authored-by: Dan Fellin <dan@highwaterlabs.com> Co-authored-by: Eric Neumann <eneumann@users.noreply.github.com> Co-authored-by: Don Chiniquy <drchiniquy@gmail.com> Co-authored-by: nahuel-polyapi <nahuel@polyapi.io> Co-authored-by: Bboydozzy96 <federico.marchini@hotovo.com> Co-authored-by: FedeMarchiniHotovo <130762770+FedeMarchiniHotovo@users.noreply.github.com> Co-authored-by: Shina Akinboboye <shina@polyapi.io> Co-authored-by: Richard <github.agonize754@passmail.net> Co-authored-by: Shina Akinboboye <60622084+akinboboye@users.noreply.github.com> Co-authored-by: Daniel-Estoll <115661842+Daniel-Estoll@users.noreply.github.com> Co-authored-by: Ashir Rao <69091220+Ash1R@users.noreply.github.com> Co-authored-by: Ashir Rao <ar2558@cornell.edu> Co-authored-by: eric.neumann <neumann.bend@gmail.com>
Should cache arguments and only rewrite them when there are less restrictive args.
function id and names weren't arguments, I think, so I added those as arguments to generate.