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

several changes #127

Merged
merged 9 commits into from
May 5, 2023
Merged

several changes #127

merged 9 commits into from
May 5, 2023

Conversation

Tieske
Copy link
Contributor

@Tieske Tieske commented Feb 19, 2023

  • cleanup error template
  • add redirect method
  • add sendFile method (download file) + plugin
  • add a TLS plugin

closes #51
closes #36

@Tieske Tieske force-pushed the updates branch 3 times, most recently from fa70e70 to 4f06b81 Compare February 22, 2023 16:50
@EvandroLG
Copy link
Owner

It's just an amazing contribution! It'll be the perfect release of the Pegasus v1! 👍

@Tieske
Copy link
Contributor Author

Tieske commented Mar 5, 2023

I have a few more changes to make before a 1.0, but it’s shaping up nicely

@EvandroLG
Copy link
Owner

It looks great @Tieske!
Would you like to add something else to this PR? Otherwise, it seems to me the perfect release of the Pegasus v1!

@Tieske
Copy link
Contributor Author

Tieske commented Mar 28, 2023

preferably I'd like #131 as well in the 1.0 version.

But I'd like your opinion on the following; should the router be a plugin, or should the router replace the handler? Personally I think the latter, since each router-path should probably get its own plugins.

@EvandroLG
Copy link
Owner

@Tieske IMO it's not the responsibility of the HTTP server to implement the router. Ideally, a framework or plugin should be responsible for a router implementation as it's more of a high-level thing. I'm open to changing my opinion, though :)
Do you remember any server (Python, Node, ngnix, Apache, etc) that natively implements a router logic?

@Tieske
Copy link
Contributor Author

Tieske commented Apr 6, 2023

Lapis is an example.

with a router you typically want specific plugins on specific paths. Eg.

  • /downloads/... should get a plugin that serves files as downloads
  • /static/... should get a plugin that serves files as html/text
  • /api/... should have plugins for content negotiation (if I request /api/user/evandrolg I might want to serve JSON if requested by an app, or an html-div if by a browser)
  • all of the above should probably have the "compression" plugin

Now how would this work if the router itself is a plugin?

If the router is implemented in the handler, then it could simply default to a route "/" with the "files" plugin first, and a callback as fallback. That would retain the current handler functionality.

@EvandroLG
Copy link
Owner

@Tieske IMO Lapis is not the best example, as they are not an HTTP server but a web framework.
Of course, a web framework like Lapis, Rails or Django should implement its router solution, but I don't see good reasons to have it as a native feature of an HTTP server like Pegasus. IMO, The responsibility of an HTTP server is to implement the server part of the HTTP and HTTPS protocols.

@Tieske
Copy link
Contributor Author

Tieske commented Apr 17, 2023

The responsibility of an HTTP server is to implement the server part of the HTTP and HTTPS protocols.

Good point.

So that means that a router should be either an additional handler implementation (instead of a replacement) within the pegasus project, or a separate project altogether (an external "handler" override).

Would the former be acceptable; a second "handler" implementing a router? if not, how did you envision #50 to be implemented?

@EvandroLG
Copy link
Owner

@Tieske I think we could make it available as part of the plugin structure of Pegasus. WDYT? Would you happen to have any other suggestions for it?

@Tieske
Copy link
Contributor Author

Tieske commented Apr 24, 2023

can you elaborate on what you mean by "we could make it available as part of the plugin structure of Pegasus"?

Copy link
Owner

@EvandroLG EvandroLG left a comment

Choose a reason for hiding this comment

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

LGTM :)

@EvandroLG EvandroLG merged commit b856757 into EvandroLG:master May 5, 2023
@EvandroLG
Copy link
Owner

@Tieske, let's move the discussion about the Router to the proper PR.
Thanks again! :)

@Tieske Tieske deleted the updates branch May 5, 2023 12:28
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.

Create redirect method Create HTTPS plugin
2 participants