-
Notifications
You must be signed in to change notification settings - Fork 597
Convert files to data URLs on webhook post #2185
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
base: main
Are you sure you want to change the base?
Changes from all commits
0ff893d
4a3201b
dbcfb2a
8707aba
8bca3d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,9 +1,12 @@ | ||||||
import tempfile | ||||||
|
||||||
import requests | ||||||
import responses | ||||||
from responses import registries | ||||||
|
||||||
from cog.schema import PredictionResponse, Status, WebhookEvent | ||||||
from cog.server.webhook import webhook_caller, webhook_caller_filtered | ||||||
from cog.types import Path | ||||||
|
||||||
|
||||||
@responses.activate | ||||||
|
@@ -153,3 +156,28 @@ def test_webhook_caller_connection_errors(): | |||||
c = webhook_caller("https://example.com/webhook/123") | ||||||
# this should not raise an error | ||||||
c(response) | ||||||
|
||||||
|
||||||
@responses.activate | ||||||
def test_webhook_caller_converts_files_data_urls(): | ||||||
with tempfile.NamedTemporaryFile() as tmpfile: | ||||||
with open(tmpfile.name, "w", encoding="utf8") as handle: | ||||||
handle.write("hello world") | ||||||
|
||||||
c = webhook_caller("https://example.com/webhook/123") | ||||||
|
||||||
payload = { | ||||||
"status": Status.SUCCEEDED, | ||||||
"output": Path(tmpfile.name), | ||||||
"input": {}, | ||||||
} | ||||||
response = PredictionResponse(**payload) | ||||||
payload["output"] = "data:None;base64,aGVsbG8gd29ybGQ=" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, what is this test doing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It tests that the webhook is sent data URLs rather than tmp file directories. I can change the mime-type however it seems odd to me that the |
||||||
|
||||||
responses.post( | ||||||
"https://example.com/webhook/123", | ||||||
json=payload, | ||||||
status=200, | ||||||
) | ||||||
|
||||||
c(response) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok just for my understanding, without the change we would've passed back the path to the
/tmp
file which then the caller could not access, so we're uploading the file (somewhere) and passing a URL back? And then the line below has a b64 encoded URL