Skip to content

Conversation

prawnsalad
Copy link
Member

No description provided.

@prawnsalad
Copy link
Member Author

Looking good so far, some points:

  1. All outgoing HTTP requests need a timeout, otherwise they could get stalled and eventually tag WIG down
  2. The noembed fallback should probably be configurable so that if noembed ever breaks we can use something else? Perhaps a FallbackOEmbedURL config option along with the optional JSON keys needed to be read from the response.
  3. Can we rename the Embed to something more specific, maybe WebPreview?
  4. The external files (template, oembed providers) do have a big drawback that WIG is no longer a single binary which has always been a selling point for many. Looking into packaging these within the binary would be helpful so it can first check for these files somewhere, then if not found, use the internal packaged versions. I"m pretty sure I've seen some golang specific way to package templates into the binary before.

@prawnsalad
Copy link
Member Author

Adding to the above, now that we have a kiwi preview page, a standalone preview page would be very nice. Basically the exact same template but designed to run directly within the browser. Perhaps the current kiwi preview URL could accept a ?embed=no URL param which switches between the 2 templates?

This could then be a starting block for future additions of uploading files via this page directly.

@prawnsalad
Copy link
Member Author

Trying to debug why some things wern't working is difficult. Some extra info would be very useful:

  • If the fallback oEmbed provider includes an error property, report this error Fallback oEmbed provider returned an error, "the error message"
  • ERR Unexpected error in noEmbed error="Failed to get target json key" this isn't clear. Maybe Fallback oEmbed provider did not include a JSON property of "the_configged_property"?

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