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

Pretty difficult to use #1795

Open
roy-work opened this issue Dec 8, 2023 · 2 comments
Open

Pretty difficult to use #1795

roy-work opened this issue Dec 8, 2023 · 2 comments

Comments

@roy-work
Copy link

roy-work commented Dec 8, 2023

Describe the bug
This API is just not great.

I needed to write a script to just find & replace one Slack channel that was our notifications on monitors to another. This was … difficult.

I'm going to summarize a number of usability issues here; filing a bug for each would be onerous.

  • SyntheticsApi includes both a patch_test, and an update_api_test. I have absolutely no idea what the difference between "patch" and "update" is, and the docs don't tell me.

  • AFAICT, SyntheticsApi.patch_test (as documented here) is a lie? I have the latest version of the library, and AFAICT, that, and the associated types, just don't exist.

  • If you SyntheticsApi.list_tests, you get, essentially, a list of SyntheticsTestDetails. (It is wrapped by a pointless wrapper object, but that's whatever.) However, update_api_test requires a SyntheticsAPITest, so you can't pass the result to it!

  • If you call get_api_test, however, you'll get the right type: a SyntheticsAPITest! But attempting to then call update_api_test with that results in a runtime error:

    HTTP response body: {'errors': ["Additional properties are not allowed ('modified_at', 'creator', 'monitor_id', 'created_at' were unexpected)"]}
    

    I attempted a number of things to attempt to remove those properties form the object, but none of which worked. … the entire point of a type system is that if you function declares itself to take a Foo, then you should be able to pass it instances of Foo. If you can't, you have foo instances that aren't really Foo.

  • Setting host in Configuration causes credentials to be sent in the clear #1794, which was severe enough that I have split it out to a separate bug.

  • authn issues appear to return 403, which sent me down the wrong paths for a bit.

  • Many of the types in the library, when repr'd, do not emit faithful reprs. For example, if you repr a monitor, you'll get,

    …
     'type': 'synthetics alert'}
    

    This led to me writing bugs such as,

    monitor['type'] == 'synthetics alert'
    

    Which is False (!) for the above. That's because the value for 'type' there is not truly a string, it's a MonitorType. The repr here is just … lying.

    Python's has built-in support for enumerations in the form of the enum module, and it won't do stuff like this that will leave one pulling out one's hair mid debugging session.

    (Even the outer type there appears to be a dict when repr()'d … but its not.)

Environment and Versions (please complete the following information):
A clear and precise description of your setup:

  • version for this project in use: 2.19.0 (latest, as of the time of writing)

Additional context
As you might be able to tell from the above, most of my frustration was around the synthetics portion. I was actually successful at updating non-synthetics monitors — except for running into errors with monitors that were actually synthetics (another Liskov substitutability violation, really), and then attempting to update those with SyntheticsApi. I eventually gave up, and just did it by hand … we don't have that many synthetics.

@therve
Copy link
Contributor

therve commented Dec 11, 2023

Hi,

Unfortunately, most if not all the issues are API related, not the library. Regarding patch_test, it's a new method to change a test that doesn't require to pass all the data, It's not a lie, it's in master but not released yet.
We can probably tweak enums to have equality works.
Thanks.

Copy link

Thanks for your contribution!

This issue has been automatically marked as stale because it has not had activity in the last 30 days. Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity. Thank you for participating in the Datadog open source community.

If you would like this issue to remain open:

  1. Verify that you can still reproduce the issue in the latest version of this project.

  2. Comment that the issue is still reproducible and include updated details requested in the issue template.

@github-actions github-actions bot added the stale label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants