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

[IO-1046][IO-1035] Team method upates and lazy loading #598

Merged
merged 8 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions darwin/future/core/backend.py

This file was deleted.

33 changes: 32 additions & 1 deletion darwin/future/data_objects/team.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
from typing import List, Optional
from __future__ import annotations

from typing import List, Optional, Tuple

from pydantic import validator

# from darwin.future.core.backend import get_team
Nathanjp91 marked this conversation as resolved.
Show resolved Hide resolved
from darwin.future.core.client import Client
from darwin.future.data_objects.dataset import DatasetList
from darwin.future.data_objects.team_member_role import TeamMemberRole
from darwin.future.data_objects.validators import parse_name
Expand Down Expand Up @@ -51,5 +55,32 @@ class Team(DefaultDarwin):
# Data Validation
_slug_validator = validator("slug", allow_reuse=True)(parse_name)

@staticmethod
def from_client(client: Client, team_slug: Optional[str] = None) -> Team:
"""Returns the team with the given slug"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we do these with full docblocks, so they can be properly doc'd when it comes to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I'll go back and write up docs for all the stuff I've added

if not team_slug:
team_slug = client.config.default_team
return get_team(client, team_slug)


TeamList = List[Team]


def get_team(client: Client, team_slug: Optional[str] = None) -> Team:
"""Returns the team with the given slug"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment

if not team_slug:
team_slug = client.config.default_team
response = client.get(f"/teams/{team_slug}/")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're starting to get a mix of functions that use the monad exceptions, return response, and those that raise. Not something for now, but we should work out what our approach is for using one vs the other.

Copy link
Contributor Author

@Nathanjp91 Nathanjp91 Jun 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for me it comes down to when we get a single object back or a list of objects. If we're only returning a single 'block', ie something that's all related to each other and if one thing goes wrong it's all poisoned, then it makes sense to just return the object + raise if any issues, but if we have a function that returns a list of objects and they're all separate, then it makes sense to package up the good data and return it with the exceptions monad style.

return Team.parse_obj(response)


def get_team_members(client: Client) -> Tuple[List[TeamMember], List[Exception]]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then this one, sorry!

response = client.get(f"/memberships")
members = []
errors = []
for item in response:
try:
members.append(TeamMember.parse_obj(item))
except Exception as e:
errors.append(e)
return (members, errors)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think the brackets are unnecessary, although totally stylistic, and your choice on this one.

3 changes: 1 addition & 2 deletions darwin/future/meta/queries/team_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@

from typing import Any, Dict, List

from darwin.future.core.backend import get_team_members
from darwin.future.core.client import Client
from darwin.future.core.types.query import Query, QueryFilter
from darwin.future.data_objects.team import TeamMember
from darwin.future.data_objects.team import TeamMember, get_team_members

Param = Dict[str, Any] # type: ignore

Expand Down
3 changes: 2 additions & 1 deletion darwin/future/tests/core/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import pytest

from darwin.future.core.client import Client, DarwinConfig
from darwin.future.data_objects.team import Team, TeamMember, TeamMemberRole
from darwin.future.data_objects.team import Team, TeamMember
from darwin.future.data_objects.team_member_role import TeamMemberRole


@pytest.fixture
Expand Down
30 changes: 30 additions & 0 deletions darwin/future/tests/data_objects/fixtures.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import pytest


@pytest.fixture
def basic_team() -> dict:
return {"slug": "test-team", "id": 0}


@pytest.fixture
def basic_dataset() -> dict:
return {"name": "test-dataset", "slug": "test-dataset"}


@pytest.fixture
def basic_release() -> dict:
return {"name": "test-release"}


@pytest.fixture
def basic_combined(basic_team: dict, basic_dataset: dict, basic_release: dict) -> dict:
combined = basic_team
combined["datasets"] = [basic_dataset]
combined["datasets"][0]["releases"] = [basic_release]
return combined


@pytest.fixture
def broken_combined(basic_combined: dict) -> dict:
del basic_combined["datasets"][0]["name"]
return basic_combined
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,7 @@
from darwin.future.data_objects.dataset import Dataset
from darwin.future.data_objects.release import Release
from darwin.future.data_objects.team import Team


@pytest.fixture
def basic_team() -> dict:
return {"slug": "test-team", "id": 0}


@pytest.fixture
def basic_dataset() -> dict:
return {"name": "test-dataset", "slug": "test-dataset"}


@pytest.fixture
def basic_release() -> dict:
return {"name": "test-release"}


@pytest.fixture
def basic_combined(basic_team: dict, basic_dataset: dict, basic_release: dict) -> dict:
combined = basic_team
combined["datasets"] = [basic_dataset]
combined["datasets"][0]["releases"] = [basic_release]
return combined


@pytest.fixture
def broken_combined(basic_combined: dict) -> dict:
del basic_combined["datasets"][0]["name"]
return basic_combined
from darwin.future.tests.data_objects.fixtures import *


def test_integrated_parsing_works_with_raw(basic_combined: dict) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
import responses
from pydantic import ValidationError

from darwin.future.core import backend as be
from darwin.future.core.client import Client
from darwin.future.data_objects.team import Team, TeamMember
from darwin.future.data_objects.team import Team, TeamMember, get_team, get_team_members
from darwin.future.tests.core.fixtures import *
from darwin.future.tests.fixtures import *

Expand All @@ -17,7 +16,7 @@ def test_get_team_returns_valid_team(base_client: Client, base_team_json: dict,
with responses.RequestsMock() as rsps:
rsps.add(responses.GET, endpoint, json=base_team_json)

team = be.get_team(base_client, slug)
team = get_team(base_client, slug)
assert team == base_team


Expand All @@ -28,7 +27,7 @@ def test_get_team_fails_on_incorrect_input(base_client: Client, base_team: Team)
rsps.add(responses.GET, endpoint, json={})

with pytest.raises(ValidationError):
team = be.get_team(base_client, slug)
team = get_team(base_client, slug)


def test_get_team_members_returns_valid_list(base_client: Client, base_team_member_json: dict) -> None:
Expand All @@ -37,7 +36,7 @@ def test_get_team_members_returns_valid_list(base_client: Client, base_team_memb
with responses.RequestsMock() as rsps:
rsps.add(responses.GET, endpoint, json=[base_team_member_json, base_team_member_json])

members, errors = be.get_team_members(base_client)
members, errors = get_team_members(base_client)
assert len(members) == 2
assert len(errors) == 0
assert members == synthetic_list
Expand All @@ -48,8 +47,20 @@ def test_get_team_members_fails_on_incorrect_input(base_client: Client, base_tea
with responses.RequestsMock() as rsps:
rsps.add(responses.GET, endpoint, json=[base_team_member_json, {}])

members, errors = be.get_team_members(base_client)
members, errors = get_team_members(base_client)
assert len(members) == 1
assert len(errors) == 1
assert isinstance(errors[0], ValidationError)
assert isinstance(members[0], TeamMember)


def test_team_from_client(base_client: Client, base_team_json: dict, base_team: Team) -> None:
with responses.RequestsMock() as rsps:
rsps.add(
responses.GET,
base_client.config.api_endpoint + f"teams/{base_client.config.default_team}",
json=base_team_json,
)

team = Team.from_client(base_client)
assert team == base_team