-
Notifications
You must be signed in to change notification settings - Fork 2
Fork changes #8
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?
Fork changes #8
Conversation
src/BaseAssets/FileSystem/Certs.mo
Outdated
| // update the status code to '304 Not Modified' | ||
| get_success_endpoint(key_or_alias, encoding, headers_array, is_aliased).status(304); | ||
| // update the status code to '304 Not Modified' and remove the body since the content is not sent | ||
| get_success_endpoint(key_or_alias, encoding, headers_array, is_aliased).status(304).body(""); |
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.
@tomijaga I was not able to get cached assets back that were certified because the body was expected to be populated even though it was a 304
| }; | ||
|
|
||
| public func get_html_alias_root(fs : T.FileSystem, alias : Text) : ?Text { | ||
| if (not Text.endsWith(alias, #text ".html")) return null; |
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.
@tomijaga This an a few other changes were added to TRY to make it so that when i add an asset called index.html, i would be able to get a certified form from /, but instead i would always get the index.html, but certified for the /index.html path, vs the / path
I don't think i was successful, and this is my biggest issue right now
From what i could figure out, if you added a file as /, it would resolve to /index.html, but not the other way around (i could be wrong, but thats what it looks like)
src/BaseAssets/Http.mo
Outdated
| let (status_code, body, opt_body_hash) : (Nat16, Blob, ?Blob) = if (contains_hash) { | ||
| (304, "", null); | ||
| } else { | ||
| let content_chunk : Blob = switch (Encoding.get_chunk(self.fs, encoding, chunk_index)) { |
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.
Optimization since 304 didn't need content
src/BaseAssets/Http.mo
Outdated
|
|
||
| public func http_request_streaming_callback(self : T.StableStore, token : T.StreamingToken) : T.Result<T.StreamingCallbackResponse, Text> { | ||
| public func http_request_streaming_callback(self : T.StableStore, rawToken : T.StreamingToken) : T.Result<T.StreamingCallbackResponse, Text> { | ||
| let ?t : ?T.CustomStreamingToken = from_candid (rawToken) else return #err("http_request_streaming_callback(): Invalid token"); |
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.
I had changed all the signatures to have the streaming token just a Blob vs that specific data structure
|
Thanks for these. I'll take a look later and merge the updates. |
Here are the changes that i did in the fork that fixed some issues i was having. Ill comment on them because some should be ignored.
Just doing a draft PR so i can comment on the changes, don't intend to merge