Use the v1 version of the log api

[naddeoa]

This switches from the v0 to the v1 profile upload api, replacing the previous use of log_async in the whylabs client with log_profile. The apis are functionally the same but the dataset id and the org id are specified differently. The dataset id has to be supplied via a different name and ends up as a header in the request, while the org id is not specified at all because its derived from the api key.

In terms of implementation, I tried to do this all from outside of whylogs to avoid having to make this change but there are several issues that pop up when you try to pass in your own WhylabsClient, and especially your own ApiClient that made that impossible. The refactor looks daunting as well.

Instead, this change just passes on the WhylabsClient to the other "sub writers" that are secretly made in the whylabs writer, and hard codes the use of the v1 api over the v0 api.

[richard-rogers]

Some of the integration tests are failing. Investigating...

[richard-rogers]

Some of the integration tests are failing. Investigating...

Some just look like the mocks for injecting throttling need to be adjusted for V1.
More concerning, some fail to retrieve the uploaded profiles. That could just be the platform running slow or something more serious.

There are also some transaction test failures I haven't looked at yet.

[richard-rogers]

There are integration tests failing on mainline too, so maybe some of this is a backend problem.

[naddeoa]

@richard-rogers Things are all green now so I guess you reran them?

[richard-rogers]

@richard-rogers Things are all green now so I guess you reran them?

The CI doesn't run the integration tests. They're still failing here and on mainline.

[richard-rogers]

@richard-rogers Things are all green now so I guess you reran them?

The CI doesn't run the integration tests. They're still failing here and on mainline.

Mainline CI is passing now. I updated the URI mocks, so now only a few transaction tests are failing.

[naddeoa]

@richard-rogers can you post the failures or something so I can debug?

[richard-rogers]

@richard-rogers can you post the failures or something so I can debug?

FAILED tests/api/writer/test_whylabs_integration.py::test_transaction_aborted - AssertionError: assert '<ExceptionIn...ntextmanager>' == 'Transaction has been aborted'
FAILED tests/api/writer/test_whylabs_integration.py::test_old_transaction_context - AssertionError: assert 1 == 7
FAILED tests/api/writer/test_whylabs_integration.py::test_transaction_distributed - IndexError: list index out of range

I will investigate them today. You can run them by setting up the WHYLABS_* environment variables and running poetry run pytest tests/api/writer/test_whylabs_integration.py --load

[richard-rogers]

I will investigate them today. You can run them by setting up the WHYLABS_* environment variables and running poetry run pytest tests/api/writer/test_whylabs_integration.py --load

I think I figured it out. The WhyLabsTransactionWriter ctor used to always generate a new transaction id and stick it into its new WhyLabsClient. With this PR, the transaction writer is sharing the client with the parent writer, so the actual transaction id gets stomped on. I think there's a relatively easy fix, but I'd really like to clean up the writer code more in whylogs 2.0.

[richard-rogers]

Do we want to leave this as a dev dependency?

[naddeoa]

nope, I'll update this now and push again. Just a timing issue with the platform release

[richard-rogers]

Integration tests are green now