Global Config #1045

collindutter · 2024-08-06T19:37:12Z

I have read and agree to the contributing guidelines for submitting new pull requests.

Describe your changes

This change completely refactors how configuration is handled in the framework. Since the beginning we have relied on Structure to serve as the source of truth for configuration values. However, as the framework grew in complexity, it became increasingly difficult (and sometimes wrong) to propagate a Structure reference around in order to access these configuration values.

This PR introduces a global singleton, griptape.config.config, that stores these configuration values. Today, it stores configuration for drivers (previously called StructureConfig), and logging. The framework has been updated to pull from this object for default values.

Since this configuration now lives in the config object, I have removed these configuration values from Structure. Note that Agent.prompt_driver and Agent.stream were kept since the purpose of an Agent is to provide a syntactically concise way to create a Structure with either a PromptTask/ToolkitTask. These fields felt consistent with the other helper field: Agent.tools.

Overall I really like this change, but you might notice that not all drivers are included in DriversConfig. Does it makes sense to provide a default value for something like a WebScraperDriver? Or should this be left to the specific areas of the framework that require it? We could just provide a None default, but this may mask the fact that something like the WebSearch Tool does actually require one.

Changelog

Added:

Global config, griptape.config.config, for setting global configuration defaults.

Changed:

BREAKING: Removed Pipeline.prompt_driver and Workflow.prompt_driver. Agent.prompt_driver has not been removed.
BREAKING: Removed Pipeline.stream and Workflow.stream. Agent.stream has not been removed.
BREAKING: Removed Structure.embedding_driver, set this via griptape.config.config.drivers.embedding instead.
BREAKING: Removed Structure.custom_logger and Structure.logger_level, set these via griptape.config.config.logger instead.
Engines that previously required Drivers now pull from griptape.config.config.drivers by default.
BREAKING: Removed BaseStructureConfig.merge_config.
BREAKING: Renamed StructureConfig to DriverConfig, and renamed fields accordingly.

Issue ticket number and link

Closes #826 (merge_config is removed).

📚 Documentation preview 📚: https://griptape--1045.org.readthedocs.build//1045/

codecov · 2024-08-06T20:34:52Z

Codecov Report

Attention: Patch coverage is 94.19643% with 13 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
griptape/utils/chat.py	0.00%	5 Missing ⚠️
griptape/tasks/actions_subtask.py	60.00%	4 Missing ⚠️
griptape/config/logging_config.py	92.30%	0 Missing and 1 partial ⚠️
...ptape/memory/task/storage/text_artifact_storage.py	66.66%	0 Missing and 1 partial ⚠️
griptape/tasks/base_image_generation_task.py	75.00%	1 Missing ⚠️
griptape/tasks/rag_task.py	66.66%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Event listners

vachillo · 2024-08-08T20:10:16Z

can you write up the motivation for these changes and an example? hard to look at 123 files that seems like mostly small config changes and formatting

collindutter · 2024-08-09T16:38:52Z

@vachillo updated the PR description with more context

griptape/utils/chat.py

griptape/utils/stream.py

dylanholmes

Couple of comments, but they can be addressed in another PR if at all. No more revisions on this plz

dylanholmes · 2024-08-12T17:55:38Z

CHANGELOG.md


 ### Changed
 - **BREAKING**: Removed all uses of `EventPublisherMixin` in favor of `event_bus`.
 - **BREAKING**: Removed `EventPublisherMixin`.
+- **BREAKING**: Removed `Pipeline.prompt_driver` and `Workflow.prompt_driver`. `Agent.prompt_driver` has not been removed.


What's special about Agent.prompt_driver ?

From the PR description:

Note that Agent.prompt_driver and Agent.stream were kept since the purpose of an Agent is to provide a syntactically concise way to create a Structure with either a PromptTask/ToolkitTask. These fields felt consistent with the other helper field: Agent.tools.

Let me know if this is not a sufficient explanation, happy to go into more detail.

dylanholmes · 2024-08-12T17:55:59Z

CHANGELOG.md


 ### Changed
 - **BREAKING**: Removed all uses of `EventPublisherMixin` in favor of `event_bus`.
 - **BREAKING**: Removed `EventPublisherMixin`.
+- **BREAKING**: Removed `Pipeline.prompt_driver` and `Workflow.prompt_driver`. `Agent.prompt_driver` has not been removed.
+- **BREAKING**: Removed `Pipeline.stream` and `Workflow.stream`. `Agent.stream` has not been removed.


What's special about Agent.stream?

Replied above.

dylanholmes · 2024-08-12T17:58:05Z

docs/griptape-framework/drivers/embedding-drivers.md

-Here is how you can override the Embedding Driver that is used by default in Structures. 
+Here is how you can override the Embedding Driver that is used by default in Structures.

 ```python
 --8<-- "docs/griptape-framework/drivers/src/embedding_drivers_10.py"


Separating the snippets from the docs makes proof reading more difficult

Agreed, but the type checking/linting was essential during development.

dylanholmes · 2024-08-12T18:01:16Z

docs/griptape-framework/misc/src/events_3.py

+config.drivers = OpenAiDriverConfig(prompt=OpenAiChatPromptDriver(model="gpt-4o", stream=True))
+
+pipeline = Pipeline()


It seems weird that we are "setting" the stream flag at this level. (like you don't know where it is going to be used yet and it could be used for streaming or not). Is changing that outside of the scope of this PR? If so, let's create a task to change it

Agreed, I just moved the location to a more natural location.

dylanholmes · 2024-08-13T15:11:42Z

griptape/structures/agent.py

-                task = ToolkitTask(self.input, tools=self.tools, max_meta_memory_entries=self.max_meta_memory_entries)
+                task = ToolkitTask(
+                    self.input,
+                    prompt_driver=self.prompt_driver,


Why not take this from config too?

It does. self.prompt_driver defaults to config.drivers.prompt

I guess I mean, why do we need prompt_driver on agent at all?

We technically don't, it's just a convenience field to avoid dealing with the configuration object.

dylanholmes · 2024-08-13T15:12:03Z

griptape/structures/agent.py

            else:
-                task = PromptTask(self.input, max_meta_memory_entries=self.max_meta_memory_entries)
+                task = PromptTask(
+                    self.input, prompt_driver=self.prompt_driver, max_meta_memory_entries=self.max_meta_memory_entries


Same here, why not take prompt driver from config?

Good catch, updated stream to default to config.drivers.prompt.stream.

dylanholmes

Looks fine, but I replied to some comments

dylanholmes · 2024-08-13T17:42:41Z

docs/griptape-framework/misc/src/events_3.py

 pipeline.add_tasks(
    ToolkitTask(
        "Based on https://griptape.ai, tell me what griptape is.",
+        prompt_driver=OpenAiChatPromptDriver(model="gpt-4o", stream=True),


I suppose passing stream in to the driver works, but I think its a little confusing still. I'd expect either pipeline.run(stream=True) or maybe Pipeline(stream=True)...

A Pipeline may not contain any Tasks that use Prompt Drivers therefore streaming would be irrelevant. For Agents, it's always either be a Prompt Task or Toolkit Task (unless a user overrides tasks).

dylanholmes · 2024-08-13T17:44:03Z

griptape/structures/agent.py

-                task = ToolkitTask(self.input, tools=self.tools, max_meta_memory_entries=self.max_meta_memory_entries)
+                task = ToolkitTask(
+                    self.input,
+                    prompt_driver=self.prompt_driver,


I guess I mean, why do we need prompt_driver on agent at all?

collindutter added 16 commits August 7, 2024 16:42

Add global event bus

adf4836

WIP

ae20c82

WIP

44f9ebd

Event listners

Fix tests

6b76237

Namespace config

e8c1fff

Rename driver config fields

514665f

Revert changelog update

ef24d49

Rename Structure Config to Driver Config

ab65783

Rename doc fields

ecfa358

Move events into config

582f55c

Add back util fields for Agent

e80f78e

Revert changelog replaces

91619a5

Fix type

9ebd44e

Revert some of agent test

b7e1359

Add logging module to config

97564a2

Fix bad rebase

4220e0f

collindutter force-pushed the refactor/global-config branch from e7cf26b to 4220e0f Compare August 7, 2024 23:57

collindutter changed the base branch from dev to feature/event-bus August 7, 2024 23:57

collindutter mentioned this pull request Aug 8, 2024

Fix Conversation Memory Autoload #1033

Closed

collindutter added 5 commits August 8, 2024 09:47

Update docs

f8a616f

Add global event bus

95d83b5

Update docs

951a4ed

Make event listeners private

025437b

Rename EventBus to event_bus

0f19385

collindutter force-pushed the feature/event-bus branch from 50caaec to 0f19385 Compare August 8, 2024 17:19

collindutter added 4 commits August 8, 2024 10:29

Fix doc

729f3aa

Fix test

c391b14

Rename event bus

7baefac

Merge branch 'feature/event-bus' into refactor/global-config

98fa12d

Update changelog

daa1710

collindutter force-pushed the refactor/global-config branch from fc518f7 to daa1710 Compare August 8, 2024 19:52

collindutter marked this pull request as ready for review August 8, 2024 20:04

collindutter changed the title ~~Refactor/global config~~ Global Config Aug 8, 2024

collindutter added 2 commits August 9, 2024 09:09

Merge branch 'dev' into refactor/global-config

8962648

Fix type errors

11f9ac8

collindutter requested a review from vachillo August 9, 2024 16:38

vachillo reviewed Aug 12, 2024

View reviewed changes

griptape/utils/chat.py Outdated Show resolved Hide resolved

griptape/utils/chat.py Show resolved Hide resolved

griptape/utils/stream.py Outdated Show resolved Hide resolved

collindutter added 3 commits August 12, 2024 11:26

Merge branch 'dev' into refactor/global-config

952e07f

Merge branch 'dev' into refactor/global-config

f3c908d

Fix utilities checking for stream

d0fab25

collindutter requested a review from vachillo August 12, 2024 18:56

vachillo reviewed Aug 12, 2024

View reviewed changes

griptape/utils/stream.py Show resolved Hide resolved

dylanholmes previously approved these changes Aug 13, 2024

View reviewed changes

collindutter mentioned this pull request Aug 13, 2024

Add logging filters, update config namespaces #1059

Merged

1 task

vachillo previously approved these changes Aug 13, 2024

View reviewed changes

Clean up example

1ff7e88

collindutter dismissed stale reviews from vachillo and dylanholmes via 1ff7e88 August 13, 2024 17:36

collindutter requested a review from dylanholmes August 13, 2024 17:38

dylanholmes previously approved these changes Aug 13, 2024

View reviewed changes

Default stream to config value

d179821

collindutter dismissed dylanholmes’s stale review via d179821 August 13, 2024 17:50

collindutter force-pushed the refactor/global-config branch from 6944ba4 to d179821 Compare August 13, 2024 17:50

collindutter requested a review from dylanholmes August 13, 2024 17:53

dylanholmes approved these changes Aug 13, 2024

View reviewed changes

collindutter merged commit e600334 into dev Aug 13, 2024
13 checks passed

collindutter deleted the refactor/global-config branch August 13, 2024 18:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Global Config #1045

Global Config #1045

collindutter commented Aug 6, 2024 •

edited

Loading

codecov bot commented Aug 6, 2024 •

edited

Loading

vachillo commented Aug 8, 2024

collindutter commented Aug 9, 2024

dylanholmes left a comment

dylanholmes Aug 12, 2024

collindutter Aug 13, 2024

dylanholmes Aug 12, 2024

collindutter Aug 13, 2024

dylanholmes Aug 12, 2024

collindutter Aug 13, 2024

dylanholmes Aug 12, 2024

collindutter Aug 13, 2024

dylanholmes Aug 13, 2024

collindutter Aug 13, 2024

dylanholmes Aug 13, 2024

collindutter Aug 13, 2024

dylanholmes Aug 13, 2024

collindutter Aug 13, 2024

dylanholmes left a comment

dylanholmes Aug 13, 2024

collindutter Aug 13, 2024

dylanholmes Aug 13, 2024

		config.drivers = OpenAiDriverConfig(prompt=OpenAiChatPromptDriver(model="gpt-4o", stream=True))

		pipeline = Pipeline()

Global Config #1045

Global Config #1045

Conversation

collindutter commented Aug 6, 2024 • edited Loading

Describe your changes

Changelog

Added:

Changed:

Issue ticket number and link

codecov bot commented Aug 6, 2024 • edited Loading

Codecov Report

vachillo commented Aug 8, 2024

collindutter commented Aug 9, 2024

dylanholmes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dylanholmes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

collindutter commented Aug 6, 2024 •

edited

Loading

codecov bot commented Aug 6, 2024 •

edited

Loading