Skip to content

Included PR 12 + Poetry + OpenAI fixes#14

Closed
zakhar-kogan wants to merge 0 commit into
BerriAI:mainfrom
zakhar-kogan:main
Closed

Included PR 12 + Poetry + OpenAI fixes#14
zakhar-kogan wants to merge 0 commit into
BerriAI:mainfrom
zakhar-kogan:main

Conversation

@zakhar-kogan

Copy link
Copy Markdown
Contributor

Needs to be tested.

  • I've included PR # 12 (as I'm not well-versed in checkouts, to be honest)
  • Added (as I get it - optional) Poetry support, it's a great way to manage dependencies + create virtualenvs, using it all day every day.
  • Fixed the OpenAI models max_tokens_to_samplemax_tokens. The first one is used in Anthropic as I get it.

@ishaan-jaff

Copy link
Copy Markdown
Contributor

cc @krrishdholakia - since this is a merge with your PR

Comment thread pyproject.toml Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is the purpose of adding poetry here ?

If it's for deploying new versions of the package i'm currently working on a github action to deploy from Github -> PyPI

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You have a great point here 🤔
Mainly for the convenience of development, but it'll primarily be used as a package. There's also a build functionality to package into .whl, but again - PyPI is preferable.

And if doing a GH action, it may well render that unneeded 🥲

P.S. The action option seems awesome!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@zakhar-kogan do you need poetry for your local development ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@zakhar-kogan do you need poetry for your local development ?

Not necessarily; I can manage without it + at least add pyproject.toml and poetry.lock to .gitignore. Should I do it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes lets do that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For tracking can we add poetry and pyproject in a new PR ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, will do that in a separate commit/PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lets do this in another PR, and no need for a gitignore

Comment thread litellm/main.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@krrishdholakia from PR #12 - I don't think we should default all users with a 10s timeout

@zakhar-kogan zakhar-kogan Jul 31, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe we could delete the default 300 token limit in PR #12? 🤔
I.e. make behavior in check with default: unlimited by default, optional limit.

PS. I'll also add the temperature setting in the next PR as I'll require it at work 😅

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

from @krrishdholakia: Reason for token limit - Anthropic requires token limits. Their defaults are 300.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Order for PRs pushing to main:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@krrishdholakia set timeout for 60s

ishaan-jaff added a commit that referenced this pull request Jul 31, 2023
Comment thread README.md Outdated
@zakhar-kogan

Copy link
Copy Markdown
Contributor Author
  • Deleted the timeout decorator + parameter for the time being
  • Expanded the .env example just in case
  • .gitignored Poetry for another PR
  • Defaulted max_tokens to 2048 🤔 Or I shouldn't have?

ishaan-berri pushed a commit that referenced this pull request May 6, 2026
…rite bypass

Exercise every state-mutating endpoint as PROXY_ADMIN_VIEW_ONLY and
confirm they all return 403:
  * POST   /v2/agents
  * PATCH  /v2/agents/{id}
  * DELETE /v2/agents/{id}
  * POST   /v2/sessions
  * DELETE /v2/sessions/{id}
  * POST   /v2/sessions/{id}/runs
  * POST   /v2/sessions/{id}/runs/{rid}/cancel
  * POST   /v2/sessions/{id}/followup

Plus a happy-path test confirming view-only admins still get
cross-tenant READ access (the whole point of the role).

Greptile P1 SECURITY regression coverage (validation #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