feat: add log() method to BuildClient (Python)

[tobice]

This wraps the new /actor-builds/:build_id/log endpoint. See API reference for more information.

[tobice]

@fnesveda I haven't added any tests as the existing suite is very limited and it'd take some investment to extend it properly.

This is something I'd actually like to look into, to get the tests at least on par with apify-client-js, but it's probably not the biggest burning issue right now.

[fnesveda]

Please also add a similar method into BuildClientAsync. Unfortunately, for technical reasons, we have to maintain the sync and async versions of the client separately, which means having to do all changes twice :-/

[fnesveda]

@fnesveda I haven't added any tests as the existing suite is very limited and it'd take some investment to extend it properly.

This is something I'd actually like to look into, to get the tests at least on par with apify-client-js, but it's probably not the biggest burning issue right now.

Yeah, that's OK, the tests here are pretty basic, so it would take quite a lot of effort to add new ones for specific endpoints. It would be nice to have proper tests for the client, but it's really not worth the effort right now.

When I was writing the Python API client, I decided not to replicate the test structure from the JS API client, because I didn't really like it - the unit tests basically have to mock the whole API server, which is a lot of duplicated code, and you have no guarantee that they're mocking it properly.

I was planning to come up with some proper testing framework, but that would mean rewriting half of the integration tests we have for the platform, and that seemed like a lot of effort too. And at that point we didn't know if the investment into Python tooling would be worth it, so we decided to not put in the effort into writing automated tests, and we just tested everything manually.

In the end, I think it was the right decision - the manual testing didn't take too much time, and we don't really do many structural changes to the API client that could cause bugs that only a full test suite in the API client would catch.

Anyway, I think the decision whether it's OK to keep the API client like this, with bare-bones tests, is now on @vdusek as the new owner of our Python tooling, what do you think?

If we ever decide to write proper tests here, I think we could take inspiration from the testing framework that's in the Apify SDK for Python, there the tests are much more detailed, and the test fixtures make writing tests quite easy, I would say.

[vdusek]

Hi there, improving the test suite for the Python client is currently not on our roadmap due to the main focus on Crawlee for Python. So it's up to you guys, if you could find time to implement some better tests here, it would indeed be awesome. Otherwise, just stay with the basic tests we currently have in place.

[tobice]

@fnesveda Added the async API variant 👍

[tobice]

Re tests, I generally don't feel comfortable submitting code without at least some test automation 😅 Manual testing is a valid option but also by definition brittle.

I also don't really have that much of a problem with mocking. For a unit test, that's fine. Just a piece of code that tells me that the new method calls the expected API endpoint and pipes back the data. For me as a new developer on the team who doesn't really understand how the client is built from within, this is already useful feedback.

With integration / e2e tests, while they are obviously preferred, maintaining a full suite on every single layer (API -> client -> SDK) is perhaps also not very economical.

Anyway, as said, this is definitely something I'd enjoy looking into (after agreeing on an approach with both of you), but it also doesn't seem like a burning issue right now.

[fnesveda]

Cool 👍

I forgot to mention it in the last review, could you also add a mention about this change to CHANGELOG.md? We unfortunately don't have automated changelog generation in the Python tooling yet.

[tobice]

Updated CHANGELOG.md.