-
Notifications
You must be signed in to change notification settings - Fork 191
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
Rework initialization, improve logging, and other refactoring #315
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
siyangqiu
requested review from
bboynton97,
areibman and
HowieG
and removed request for
bboynton97 and
areibman
July 30, 2024 02:32
siyangqiu
changed the title
DRAFT: Rework initialization, improve logging, and other refactoring
Rework initialization, improve logging, and other refactoring
Jul 30, 2024
siyangqiu
commented
Jul 30, 2024
siyangqiu
commented
Jul 30, 2024
bboynton97
reviewed
Jul 30, 2024
bboynton97
reviewed
Jul 30, 2024
bboynton97
reviewed
Jul 31, 2024
bboynton97
reviewed
Jul 31, 2024
bboynton97
reviewed
Jul 31, 2024
bboynton97
approved these changes
Jul 31, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nits but nothing blocking
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📥 Pull Request
📘 Description
This change addresses a number of common pain points with the AgentOps SDK:
The
init()
method is expected to be the first entry-point into AgentOps code.-- Unexpected behavior is extremely common when this is not the case - often happens because decorators in submodules are called when the submodules are imported.
-- AgentOps integrations cannot override the default configuration as there is no
Client()
to set configuration optionsLog messages are suppressed before
init()
is called. Some warnings messages are not revealed to users as a resultErrors messages can be confusing. The some errors are split up into multiple logs. Some errors are not descriptive enough because when the issue happens, some information is no longer available
** The Change**
Client()
now has "initialized" state variable - only set to true when everything is ready when all required info is providedClient()
constructor now only sets default configuration and does not take arguments.Client().configure()
is now used to configure the client.init()
callsconfigure()
and then initializes the client.