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

Custom Report Payload #1365

Open
ypkang opened this issue Oct 11, 2024 · 7 comments · Fixed by #1380
Open

Custom Report Payload #1365

ypkang opened this issue Oct 11, 2024 · 7 comments · Fixed by #1380
Assignees
Labels
jac cloud Issues related to the jac-cloud package

Comments

@ypkang
Copy link
Contributor

ypkang commented Oct 11, 2024

Describe the feature you'd like

  • In jac-cloud, being able to define custom report payload.
  • We need to expose an interface to jac that allow the developers to fully customize/specify the response payload of their walker API.
  • This is to support cases such as returning a binary as the response payload.

Examples

For example, to support something like this

from fastapi import FastAPI, Response
from docx import Document
import io

app = FastAPI()

@app.get("/binary-response")
async def binary_response():
    # Create a sample Word document in memory
    doc = Document()
    doc.add_heading("Sample Document", level=1)
    doc.add_paragraph("This is an example paragraph.")

    # Save document to a bytes buffer
    buffer = io.BytesIO()
    doc.save(buffer)
    buffer.seek(0)

    # Return the binary data using Response with appropriate media type
    return Response(
        content=buffer.read(),
        media_type="application/vnd.openxmlformats-officedocument.wordprocessingml.document",
        headers={"Content-Disposition": "attachment; filename=example.docx"}
    )
@ypkang
Copy link
Contributor Author

ypkang commented Oct 11, 2024

@amadolid @marsninja @ChrisIsKing I came across this need for Einstein. Basically, we need to return a binary as the response for one of the walkers.

I want to start a discussion here on what we think is the best way to support this. In Jaseci 1, I remember we support custom report status and payload but I am not sure if it was fully customizable. Perhaps we can start with jaseci 1 approach as a baseline and see if it is enough.

@amadolid could you describe how the custom report work in jaseci 1 here?

@ypkang ypkang added the jac cloud Issues related to the jac-cloud package label Oct 11, 2024
@amadolid
Copy link
Collaborator

report:custom overrides the whole response regardless if there's a report or none

report:file will return a binary stream. This will take priority over custom.

file -> custom -> normal

we can have a report with typing similar to Jaseci 1

report -> Jac.report(expr)

report:custom -> Jac.report(data, Response.Custom)

report:file -> Jac.report(file, Response.File)

my only concern on this is, how can we make it more "define" as our behavior is technically a "array of entry/exit"

What I mean by that is,
on DJango or FastAPI, the api is 1 call, 1 flow, 1 response. However, in jac, 1 call, N flows, N responses.
How can we define a Walker that will describe it only returns a single file or a custom json

Should we just based the response using

can response with exit on walker?
which is the last response

null return -> normal
return data -> custom response
return file -> file stream

@amadolid
Copy link
Collaborator

I just realized, why not just have with entry {} and with exit {} similar to @ChrisIsKing suggested but this function will be converted to __with_entry__ and __with_exit__

multiple declaration of it will just overridden by the last declaration

entry will be the very first trigger when walker is called while exit will very last trigger and will determine the actual return of the walker

This can fix the custom report and binary return.

We can discuss further on Monday

@ypkang
Copy link
Contributor Author

ypkang commented Oct 13, 2024

@amadolid I am fine with the idea that the last with exit to be able to override the report with custom payload.

@marsninja
Copy link
Contributor

To through my 2 cents in this, as long as the functionality of report can be defined in the plugin interface I'm ok with what you guys decide for jac-cloud.

@marsninja marsninja removed their assignment Oct 14, 2024
@ChrisIsKing
Copy link
Collaborator

@ypkang Just to confirm, we decided to support this by introducing report::custom and the addition of with entry/exit code blocks in the future?

@ypkang
Copy link
Contributor Author

ypkang commented Oct 15, 2024

@ChrisIsKing that is correct. The behavior is:

  1. report:custom overwrites the response of the API entirely and overwrites any other normal report
  2. In the execution context, we will keep track of both report:custom and normal report. This will allow us later on to expose them at the language level so developer can manipulate even further (in a with exit at the end of the walker execution for example).

FYI @amadolid

@amadolid amadolid linked a pull request Oct 17, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jac cloud Issues related to the jac-cloud package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants