Skip to content
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

Support include_metadata Query Parameter #153

Open
SteadBytes opened this issue Mar 20, 2019 · 2 comments
Open

Support include_metadata Query Parameter #153

SteadBytes opened this issue Mar 20, 2019 · 2 comments

Comments

@SteadBytes
Copy link
Contributor

SteadBytes commented Mar 20, 2019

Issue Summary

Keen provides an include_metadata=true query parameter to return execution metadata for ad-hoc queries (docs) which is not currently supported.

This would be useful to have as an optional kwarg for the ad-hoc query methods on KeenClient.

Example API's

Return a dictionary with result and execution_metadata keys:

>>> keen.count("purchases", timeframe="this_14_days", include_metadata=True)
{
    "result": 100,
    "execution_metadata": {
        "total_processing_time": 0.09057211875915527,
        "events_scanned": 1000,
        "properties_per_event": 4,
        "total_properties_scanned": 4000,
    },
}

Return a (result, execution_metadata) tuple:

>>> keen.count("purchases", timeframe="this_14_days", include_metadata=True)
(
    100,
    {
        "total_processing_time": 0.09057211875915527,
        "events_scanned": 1000,
        "properties_per_event": 4,
        "total_properties_scanned": 4000,
    },
)

Very happy to implement and PR for it after discussion on whether to support the feature and on the desired API 👍

@masojus
Copy link
Contributor

masojus commented Mar 20, 2019

Providing a means for retrieving execution metadata is certainly on the radar. I haven't thought about how to include it yet, but your examples aren't likely too far off from what we'd want. The optional parameter list for those queries is already fairly large so there might be some consideration for how to future-proof ourselves against having to add parameters like this to 15 call site. I'll think about it and come back with a plan.

@SteadBytes
Copy link
Contributor Author

SteadBytes commented Mar 21, 2019

@masojus Yes that's a good point, currently we would need to add the kwarg to all of those methods individually. I'll have a think as well 🤔

Would you want to hold off adding include_metadata until then? (We would benefit great from it at work that's all)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants