-
Notifications
You must be signed in to change notification settings - Fork 306
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
Accept any Iterable as tags parameter #542
base: master
Are you sure you want to change the base?
Conversation
Hmm that still wouldn't work for metrics, events, ... sent through the API directly like |
Seems to be much harder to add to API. Working on it. |
@zippolyte The tests are non-deterministic for some reason. They sometimes pass and sometimes don't. Last 2 commits had some tests pass and some fail with no code difference. |
This reverts commit b5bb534.
Hey @bharel, The test issues are weird, I'm looking into them. Seems like an issue with how we recorded the API responses or something, will let you know |
Problem comes from the fact that in the test, when transforming into a list, the order of the tags array end up different than what it was when we recorded the API requests/responses, thus preventing a match. |
Use-case wise, order shouldn't matter should it? Datadog doesn't seem to mind the order afaik. |
Yeah the order doesn't matter, it's just that we recorded the requests and responses in order to mock the real thing, and if the order changes in the replays, it won't match and the tests will fail. I added a commit to your branch to make the the matcher ignore the order for tags arrays found in JSON body payloads. The tests seem to be passing now. |
/azp run DataDog.datadogpy.integration |
Azure Pipelines successfully started running 1 pipeline(s). |
Okay, I think we're almost there. We just need to add something for the tags in metrics in the API client here: https://github.com/bharel/datadogpy/blob/50fc480f7e85ececefd99ff81d7f3825c0e8609b/datadog/api/metrics.py#L92-L103, as well as change lists to other iterables in the metrics and service checks tests and we should be good I believe |
This issue has been automatically marked as stale because it has not had activity in the last 30 days. |
Hello 👋 |
Requirements for Contributing to this repository
What does this PR do?
Allow any Iterable to be the tags parameter.
Description of the Change
I made sure throughout the program, the tags parameter will be converted to a list.
This will allow you to enter any iterable, such as sets, and tuples to the tags parameter.
Alternate Designs
Thought of converting it to a list in each and every API function. Chose to do it in a central place.
Possible Drawbacks
None that I'm aware of.
Verification Process
Datadog test suite on CI/CD should run. Tests were updated accordingly.
Additional Notes
Nope.
Review checklist (to be filled by reviewers)
changelog/
label attached. If applicable it should have thebackward-incompatible
label attached.do-not-merge/
label attached.kind/
andseverity/
labels attached at least.