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

replace 'silence' with 'verbose' option #9

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

jvmncs
Copy link
Contributor

@jvmncs jvmncs commented Feb 14, 2024

I've found that the logs are pretty hard to work with when requests come with very long messages (in my case, on the order of 7k tokens). this PR expands the log verbosity options into 3 levels:

  • verbose=0: suppress tqdm bars and 100% of log output (what the docstring says silent=True should do, although currently it doesn't suppress logs at all)
  • verbose=1: display tqdm bars and log requests with metadata only
  • verbose=2: display tqdm bars and log requests with metadata and all request params (including messages). this is roughly equivalent to current behavior of silence=False

I believe verbose=1 is a more sensible default, but kept default verbose=2 to align with the current behavior

@ctjlewis
Copy link
Contributor

ctjlewis commented Feb 14, 2024

@jvmncs shit, you gotta rebase bc of the other PR. otherwise lgtm

@ctjlewis ctjlewis self-assigned this Feb 14, 2024
@jvmncs
Copy link
Contributor Author

jvmncs commented Feb 14, 2024

ah I just merged through github UI. can redo with a proper rebase if you prefer

@ctjlewis
Copy link
Contributor

no that's OK. i might update the name of that param to loglevel: "silent" | "warn" | "verbose" or something similar.

@ctjlewis ctjlewis merged commit ac048eb into SpellcraftAI:dev Feb 14, 2024
@ctjlewis
Copy link
Contributor

ctjlewis commented Feb 14, 2024

Damn, tests are failing - i forgot to add tests as required workflow for PRs or we'd have seen it here pre-merge.

Edit:

  • actually, there's a widespread OpenAI outage;
  • AND, we had an outstanding invoice;
  • AND, we can't access GitHub secrets from the PR workflow because GitHub is concerned our token will leak.

So I will reset the branch and test manually.

@ctjlewis
Copy link
Contributor

ctjlewis commented Feb 14, 2024

Confirmed failing tests had nothing to do with your code so this will ship after I bump version.

Edit: Shipped as v1.1.2.

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