search

civisanalytics / civis-r

Civis API Client for R: https://civisanalytics.com/products/civis-platform/
Other
16 stars 12 forks source link

Add `patch` option to `get_script_fun` #207

Open crich011 opened 4 years ago

crich011 commented 4 years ago

get_script_fun admits post and list as arguments but not yet patch. This would need to add logic to ensure the user doesn't try to get a function like scripts_patch_containers_runs, which doesn't exist.

patr1ckm commented 4 years ago

Yes. I think adding a runs argument would handle this, and allow fetching of more script related functions like scripts_post_containers.

crich011 commented 4 years ago

Ah, so are you imagining adding runs as an argument option to fun_type along with adding patch as an argument option to verb, for example? In such a case, what do you imagine as the trade-off between client-end vs. user-end checking to ensure that the function it returns is valid?

For example, would trying to verify with something like exists("scripts_post_containers", where = "package:civis", mode = "function") work? (That seems like a simpler and better approach than the hacky series of asserts that I had in mind.)

patr1ckm commented 4 years ago

Ah, so are you imagining adding runs as an argument option to fun_type along with adding patch as an argument option to verb, for example?

Yes!

what do you imagine as the trade-off between client-end vs. user-end checking to ensure that the function it returns is valid?

Great q. The get call in get_script_fun returns the function in the package if it's available, and fails otherwise. Your suggestion using exists works similarly! Since the user of the function is the developer, it's on the developer to make sure that usage checks out. An end-user will experience a failure here as a cryptic error, but is a bug.

crich011 commented 4 years ago

Ahh, cool! I didn't realize that get did that! (Here I was imagining some complicated set of checks and it was already happening right under my nose!)