griptape icon indicating copy to clipboard operation
griptape copied to clipboard

Lazily initialize config drivers field

Open collindutter opened this issue 1 year ago • 7 comments

Describe your changes

Lazily initializes the config.driver field so that the OpenAiChatPromptDriver doesn't initialize itself when the user doesn't intend to use it.

Issue ticket number and link

NA

collindutter avatar Aug 14 '24 21:08 collindutter

Not sure how to unit test this because of schrodinger's initialization. Inspecting the config.driver property immediately gives it a value.

collindutter avatar Aug 14 '24 21:08 collindutter

unit tests you could inspect config._drivers ?

vachillo avatar Aug 14 '24 21:08 vachillo

Technically there is no _drivers because of the alias. I'll remove the attrs magic since users will never create a _Config directly. It'll always be used via the setters.

collindutter avatar Aug 14 '24 21:08 collindutter

Codecov Report

Attention: Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
griptape/config/config.py 95.45% 1 Missing :warning:

:loudspeaker: Thoughts on this report? Let us know!

codecov[bot] avatar Aug 14 '24 21:08 codecov[bot]

Technically there is no _drivers because of the alias. I'll remove the attrs magic since users will never create a _Config directly. It'll always be used via the setters.

the alias is just for init isnt it? the actual field still exists on the class.

vachillo avatar Aug 14 '24 21:08 vachillo

Ok I really don't know how to test this because of this root conftest which immediately initializes the value for all tests. According to Codecov we're already covered here 🤷‍♂️

collindutter avatar Aug 14 '24 21:08 collindutter

Ok I really don't know how to test this because of this root conftest which immediately initializes the value for all tests. According to Codecov we're already covered here 🤷‍♂️

what about setting it to None at the top of the test file and then importing everything? including setting os.environ["OPENAI_API_KEY"] = None

vachillo avatar Aug 14 '24 21:08 vachillo