invite_to_organization: fix InvitationOrganizationJoinEmail ctor call
#637
Conversation
ryan-williams
requested review from
JohnMoutafis,
thetorpedodog and
sgillies
|
Looks like the |
There was a problem hiding this comment.
Looks correct to me @ryan-williams ! Can you add a line about actions to the docstring and a simple test? I don't think we need much more than a new test_organization.py and a test that calls invite_to_organization(), mocking tiledb.cloud.client, so that we can be sure that we've got it wired properly and that it stays that way.
For the docstring, it's okay to be vague, like we are for the other parameters. Eventually it could be nice to include a enumeration of acceptable values (here
https://github.com/TileDB-Inc/TileDB-Cloud-API-Spec/blob/3783c171eb780c47c7c5890a3616870c2bb9e061/openapi-v1.yaml#L330, I think), but we don't need to figure out the best way to do that now.
I'm not sure about the wheel upload test... I asked a question about it in #629, which introduced the test.
This looks like we’re trying to write things to the same backend path in multiple tests and it’s stepping on each other. We (the main maintainers of this package, not necessarily you) should make sure we set up per-run unique paths for our tests. |
ryan-williams
force-pushed
the
rw/sc-54356
branch
from
5d4cefb to
c2d17a4
Compare
| # This call used to raise a ValueError. | ||
| invites.invite_to_organization( | ||
| "test_org", recipients=["[email protected]"], role="read_only" | ||
| ) |
There was a problem hiding this comment.
If invite_to_organization() uses the low-level API constructor improperly, this test will fail. That's the only thing this test checks. Wider (other module functions) and deeper tests are a different story.
[sc-54346]