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

Adds export functionality to chat, removes poetry in favor of uv, bumps Honcho version to v0.0.14/python-SDK-0.0.19 #15

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bLopata
Copy link

@bLopata bLopata commented Jan 7, 2025

Solves dev-552, dev-494, dev-561

Copy link

cloudflare-workers-and-pages bot commented Jan 7, 2025

Deploying yousim with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9756163
Status: ✅  Deploy successful!
Preview URL: https://479d6067.yousim.pages.dev
Branch Preview URL: https://ben-dev-552.yousim.pages.dev

View logs

@bLopata bLopata changed the title Adds export functionality to chat, removes poetry in favor of uv. Adds export functionality to chat, removes poetry in favor of uv, bumps Honcho version to v0.0.14/python-SDK-0.0.19 Jan 7, 2025
@bLopata bLopata requested review from VVoruganti and hyusap January 7, 2025 22:15
@bLopata bLopata marked this pull request as ready for review January 7, 2025 22:16
]

# Create a temporary file
with tempfile.NamedTemporaryFile(mode='w', delete=False, suffix='.json') as tmp:
Copy link
Collaborator

Choose a reason for hiding this comment

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

is writing to disk necessary here? i feel like this could be done in memeory

Copy link
Collaborator

Choose a reason for hiding this comment

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

or is this already in memory? i've never used this library before, if so, neat trick.

Copy link
Author

@bLopata bLopata Jan 8, 2025

Choose a reason for hiding this comment

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

From python docs
tempfile.TemporaryFile(mode='w+b', buffering=-1, encoding=None, newline=None, suffix=None, prefix=None, dir=None, *, errors=None) Return a [file-like object](https://docs.python.org/3/glossary.html#term-file-like-object) that can be used as a temporary storage area. The file is created securely, using the same rules as [mkstemp()](https://docs.python.org/3/library/tempfile.html#tempfile.mkstemp). It will be destroyed as soon as it is closed (including an implicit close when the object is garbage collected). Under Unix, the directory entry for the file is either not created at all or is removed immediately after the file is created. Other platforms do not support this; your code should not rely on a temporary file created using this function having or not having a visible name in the file system.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Output from claude about it. Maybe we should used SpooledTemporaryFile


The Python tempfile library provides both in-memory and on-disk temporary storage options. Let me explain the key functionality:

For on-disk temporary files and directories:

  • tempfile.NamedTemporaryFile(): Creates a temporary file on disk with an actual file name in the filesystem. By default, it's automatically deleted when closed.
  • tempfile.mkstemp(): Creates a temporary file on disk and returns both a file descriptor and the file path. Doesn't automatically delete the file.
  • tempfile.mkdtemp(): Creates a temporary directory on disk and returns its path.

For in-memory temporary storage:

  • tempfile.SpooledTemporaryFile(): Creates a file-like object that initially stores data in memory. If the data exceeds a size threshold (default 0.5MB), it automatically switches to using disk storage.

Here's a practical example:

import tempfile

# On-disk temporary file
with tempfile.NamedTemporaryFile() as tf:
    tf.write(b"Some data")
    tf.seek(0)
    print(tf.read())  # File is automatically deleted after this block

# In-memory with potential disk spillover
with tempfile.SpooledTemporaryFile(max_size=1024) as tf:
    tf.write(b"In memory until exceeding 1KB")
    tf.seek(0)
    print(tf.read())

The temporary files are created in your system's default temporary directory (usually /tmp on Unix-like systems or the user's temp directory on Windows). This location can be customized using the dir parameter in these functions.

Would you like me to explain any specific aspect in more detail?


services:
backend:
build:
context: .
dockerfile: Dockerfile
ports:
- "8000:8000"
- "${PORT}:${PORT}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we care about the port within the container?

Copy link
Author

Choose a reason for hiding this comment

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

I made this more complicated than needed because I was running my honcho fastAPI on (the default) port 8000, so I was specifying here to avoid any problems. I will fix this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should just stick to default setup in the compose. Can change as we need in local development, but shouldn't push it up.

@@ -1,23 +1,25 @@
[tool.poetry]
Copy link
Collaborator

Choose a reason for hiding this comment

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

good riddance haha, uv supremacy

webshell/Dockerfile Outdated Show resolved Hide resolved
RUN npm run build

# Expose the port the app runs on
# Use development server
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we use a dev server in this case?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The docker container is really used for development. In production we are just serving the static front end. the dev server is really just to make it easy to serve locally

Comment on lines +20 to +22
# Copy requirements and install dependencies
COPY requirements.txt .
RUN uv pip install --system -r requirements.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably be a uv sync command. Should follow conventions from honcho repo or on https://docs.astral.sh/uv/guides/integration/docker/


# https://stackoverflow.com/questions/29663459/python-app-does-not-print-anything-when-running-detached-in-docker
CMD ["fastapi", "run", "app.py", "--host", "0.0.0.0", "--port", "8000"]
EXPOSE ${PORT}
Copy link
Collaborator

Choose a reason for hiding this comment

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

internal port doesn't matter we can always change what port we expose on the outside. should just keep this to static port

Comment on lines +231 to +245
try:
session = honcho.apps.users.sessions.create(
app_id=honcho_app.id,
user_id=user_id,
)
except TypeError as e:
if "location_id" in str(e):
# If location_id is truly optional, try without it
session = honcho.apps.users.sessions.create(
app_id=honcho_app.id,
user_id=user_id
)
else:
raise e

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this for backwards compatability?

]

# Create a temporary file
with tempfile.NamedTemporaryFile(mode='w', delete=False, suffix='.json') as tmp:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Output from claude about it. Maybe we should used SpooledTemporaryFile


The Python tempfile library provides both in-memory and on-disk temporary storage options. Let me explain the key functionality:

For on-disk temporary files and directories:

  • tempfile.NamedTemporaryFile(): Creates a temporary file on disk with an actual file name in the filesystem. By default, it's automatically deleted when closed.
  • tempfile.mkstemp(): Creates a temporary file on disk and returns both a file descriptor and the file path. Doesn't automatically delete the file.
  • tempfile.mkdtemp(): Creates a temporary directory on disk and returns its path.

For in-memory temporary storage:

  • tempfile.SpooledTemporaryFile(): Creates a file-like object that initially stores data in memory. If the data exceeds a size threshold (default 0.5MB), it automatically switches to using disk storage.

Here's a practical example:

import tempfile

# On-disk temporary file
with tempfile.NamedTemporaryFile() as tf:
    tf.write(b"Some data")
    tf.seek(0)
    print(tf.read())  # File is automatically deleted after this block

# In-memory with potential disk spillover
with tempfile.SpooledTemporaryFile(max_size=1024) as tf:
    tf.write(b"In memory until exceeding 1KB")
    tf.seek(0)
    print(tf.read())

The temporary files are created in your system's default temporary directory (usually /tmp on Unix-like systems or the user's temp directory on Windows). This location can be customized using the dir parameter in these functions.

Would you like me to explain any specific aspect in more detail?

Comment on lines +6 to +7
{name = "vintro", email = "[email protected]"},
{name = "vineeth", email = "[email protected]"}
Copy link
Collaborator

@VVoruganti VVoruganti Jan 8, 2025

Choose a reason for hiding this comment

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

Can just change this to {name = "Plastic Labs", email = "[email protected]"}, Everyone played a part.

const contentDisposition = response.headers.get('content-disposition');
const filename = contentDisposition
? contentDisposition.split('filename=')[1].replace(/['"]/g, '')
: 'yousim_conversation.json';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we name the file based on the name of the simulation?

Comment on lines +227 to +231
sessions_page = honcho.apps.users.sessions.list(
app_id=honcho_app.id,
user_id=user_id
)
sessions = list(sessions_page)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can get a session directly with a get method

Comment on lines +234 to +243
for session in sessions:
if str(session.id) == session_id:
return str(session.id)

print("No session found with matching ID, checking legacy metadata...")
# If not found, check legacy metadata
for session in sessions:
if session.metadata and session.metadata.get("legacy_id") == session_id:
print(f"Found matching legacy session! ID: {session.id}")
return str(session.id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can do a list method and pass in a filter object to filter by legacy_id

Comment on lines +402 to +409
sessions = honcho.apps.users.sessions.list(
app_id=honcho_app.id,
user_id=current_user_id
)

for session in sessions:
if session.metadata and session.metadata.get("legacy_id") == session_id:
return await get_session_messages(session_id=str(session.id), user_id=current_user_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again can filter directly on the list call

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.

3 participants