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

[ENG-1182] Rename api.create_prompt() to get_or_create() and update doc/cookbook #60

Merged

Conversation

desaxce
Copy link
Collaborator

@desaxce desaxce commented Apr 23, 2024

https://www.loom.com/share/361ece4a20a2495d966521a7adcde7ed

Edit: Just noticed that the checksum for exact same (settings, template_messages) is different.
The issue is with our seed data not computing the checksum, so it looks like the current prompt version we have in DB differs from the newer passed as argument.

I adapted the APIs around prompts:

  • deprecated create_prompt for get_or_create_prompt
  • added id argument on get_prompt

To test with https://github.com/Chainlit/chainlit-cloud/pull/437.

  • Need to change checkSum for PromptVersion with ID 67907431-95dc-56cd-9cb2-ed657122e59f to 7627fac5400b71fabfa1798ea01cbe179737abb11101131fe24cb45236d19ac8 on staging PG (DigitalOcean)
  • Add video
  • Test get_prompt with id

I removed the exclude = "examples/" from mypy.ini because I think the syntax is incorrect: the examples directory is excluded from mypy checks only if it is without double quotes. Anyway, I don't think we should exclude examples from linting.

Choosing the right deprecated annotation

I recommend we don't rely on private packages like https://pypi.org/project/Deprecated/.
PEP 702 standardizes deprecated as part of the warnings module.
Python 3.13 implements PEP702 and older Python versions can just import it from typing_extensions.
It just lacks some version argument to tell which last version will be supported.

If users installed the Python extension on VS Code, it will automatically install Pylance which will strike through deprecated occurrences throughout the code. In the auto-completion suggestions though, deprecated methods do not show as stricken through (no indication that users shouldn't use it):
image

On Cursor.sh (latest AppImage for Linux - version 0.32.2), no luck getting the strike through: the extensions fetched from Cursor seem stale (Pylance is v2023.10.40) while VS Code offers v2024.4.1. I cannot say for sure that's the culprit, maybe something else in my setup is incorrect.


This change is Reviewable

Copy link

linear bot commented Apr 23, 2024

ENG-1182 Rename api.create_prompt() to get_or_create() and update doc/cookbook

client.create_prompt()

CRUD with versioning.

U => version bump.

  • create_prompt(name, prompt) -> a first call creates a v0
  • create_prompt(name, prompt) -> subsequent calls only return v0 (no creation, currently implicit for user)
    • rename to get_or_create / upsert
  • create_prompt(name, new_prompt) -> v1
    • create_or_increment / update_prompt
  • get_prompt(name) -> champion
  • get_prompt(id) ->
  • get_prompt(name, version)

Implement TS client too.

Check how deprecated appears in the automatically generated documentation.

For now we deprecate the create_prompt.

We expose create_prompt_lineage at the moment.

Adapt all pieces of docs (in code, in literal-docs, in cookbooks)

else:
raise ValueError("Either the `id` or the `name` must be provided.")

get_prompt.__doc__ = LiteralAPI.get_prompt.__doc__
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@laura-ham I found this way to have the async versions of the APIs point to the sync ones. I haven't updated all the APIs in case the docs differ.

Copy link
Contributor

@willydouhard willydouhard left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 7 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @laura-ham)


literalai/api/__init__.py line 2370 at r2 (raw file):

Previously, desaxce (Hugues de Saxcé) wrote…

@laura-ham I found this way to have the async versions of the APIs point to the sync ones. I haven't updated all the APIs in case the docs differ.

Nice!

Copy link
Contributor

@willydouhard willydouhard left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @laura-ham)

@desaxce desaxce merged commit 25ed5a9 into main Apr 26, 2024
3 checks passed
@desaxce desaxce deleted the hugues/eng-1182-rename-apicreate_prompt-to-get_or_create-and-update branch April 26, 2024 16:28
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