Skip to content

Conversation

InTheCloudDan
Copy link

  • Clean up unneeded comments
  • Switch token classes to be closer to AI Provider types
  • Move from variationDetail which generates an event each time to variation.

@InTheCloudDan InTheCloudDan requested a review from a team as a code owner October 15, 2024 16:55

class LDAIClient:
"""The LaunchDarkly AI SDK client object."""

def __init__(self, client: LDClient):
self.client = client

def model_config(self, key: str, context: Context, default_value: str, variables: Optional[Dict[str, Any]] = None) -> Any:
def model_config(self, key: str, context: Context, default_value: str, variables: Optional[Dict[str, Any]] = None) -> AIConfig:
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this is returning a dictionary, not an AIConfig type.

Also just noticed there is a rogue print in there.

Copy link
Author

Choose a reason for hiding this comment

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

@keelerm84 updated the logic to return an AIConfig

Daniel OBrien added 2 commits October 16, 2024 10:12
Remove tracking utils

return AI Config

Remove errant print
from typing import Callable, TypedDict
from dataclasses import dataclass

@dataclass
Copy link
Member

Choose a reason for hiding this comment

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

I don't typically see both @dataclass and TypedDict used together. Are both of these needed? If so, why not on the other types below?

Copy link
Author

Choose a reason for hiding this comment

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

@keelerm84 this is a lack of knowledge on my part. I switched them over to dataclasses in the following commit.

@InTheCloudDan InTheCloudDan merged commit 676b7d5 into main Oct 18, 2024
0 of 11 checks passed
@InTheCloudDan InTheCloudDan deleted the dob/trackingUpdates branch October 18, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants