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.

Sign up for GitHub

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

Jump to bottom

🚀 v3 Request: Merge static and filesystem middleware as static middleware #2031

Closed
3 tasks done
efectn opened this issue Aug 19, 2022 · 8 comments · Fixed by #3006
Closed
3 tasks done

🚀 v3 Request: Merge static and filesystem middleware as static middleware #2031

efectn opened this issue Aug 19, 2022 · 8 comments · Fixed by #3006
Assignees
@efectn
Labels
👍 Accepting PR 🧹 Updates v3
Milestone
v3

Comments

@efectn
Copy link
Member

efectn commented Aug 19, 2022

Feature Description

I think we should merge static and filesystem to make Fiber core simple in v3. Also there's no performance problem when we compare fasthttp.FS static and io/fs filesystem middleware. (#2027)

Additional Context (optional)

╭─efectn@tearch ~/v3 ‹v3-beta›
╰─$ wrk2 -t12 -c100 -d5s -R20000 http://127.0.0.1:3000/static
Running 5s test @ http://127.0.0.1:3000/static
  12 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.09ms    0.94ms  22.08ms   93.45%
    Req/Sec       -nan      -nan   0.00      0.00%
  99642 requests in 5.00s, 54.54MB read
Requests/sec:  19934.28
Transfer/sec:     10.91MB
╭─efectn@tearch ~/v3 ‹v3-beta›
╰─$ wrk2 -t12 -c100 -d5s -R20000 http://127.0.0.1:3000/filesystem
Running 5s test @ http://127.0.0.1:3000/filesystem
  12 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    17.42ms   33.75ms 264.19ms   88.56%
    Req/Sec       -nan      -nan   0.00      0.00%
  99428 requests in 5.00s, 53.29MB read
Requests/sec:  19892.46
Transfer/sec:     10.66MB
╭─efectn@tearch ~/v3 ‹v3-beta›
╰─$ wrk2 -t12 -c100 -d5s -R20000 http://127.0.0.1:3000/filesystem/test.txt
Running 5s test @ http://127.0.0.1:3000/filesystem/test.txt
  12 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.53ms    2.15ms  44.67ms   94.69%
    Req/Sec       -nan      -nan   0.00      0.00%
  99646 requests in 5.00s, 14.35MB read
Requests/sec:  19929.40
Transfer/sec:      2.87MB
╭─efectn@tearch ~/v3 ‹v3-beta›
╰─$ wrk2 -t12 -c100 -d5s -R20000 http://127.0.0.1:3000/static/test.txt
Running 5s test @ http://127.0.0.1:3000/static/test.txt
  12 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.10ms  837.88us  17.46ms   89.62%
    Req/Sec       -nan      -nan   0.00      0.00%
  99643 requests in 5.00s, 15.77MB read
Requests/sec:  19930.33
Transfer/sec:      3.16MB

Code Snippet (optional)

package  main

import  (
"log"
"os"

"github.com/gofiber/fiber/v3"
"github.com/gofiber/fiber/v3/middleware/filesystem"
)

func  main()  {
	app  := fiber.New()

	app.Use("/filesystem", filesystem.New(filesystem.Config{
		Root: os.DirFS("proxy"),
		Browse:  true,
	}))

	app.Static("/static",  "proxy", fiber.Static{
		Browse:  true,
	})
	log.Fatal(app.Listen(":3000"))
}

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have checked for existing issues that describe my suggestion prior to opening this one.
  • I understand that improperly formatted feature requests may be closed without explanation.
@leopku
Copy link

leopku commented Sep 19, 2022

How to deal with embed.FS after merging?

@efectn
Copy link
Member Author

efectn commented Sep 19, 2022 •
edited

How to deal with embed.FS after merging?

embed.FS is compatible with fs.FS (io/fs) interfaces. So it can be used without extra effort. So it'll be easier to use fs interfaces with #2027

Fasthttp FS is not well with fs interfaces but it may be improved in the future. valyala/fasthttp#1374 Perhaps we can remove filesystem middleware in v3 cuz of there will no pros over fasthttp FS after this improvement.

@leopku
Copy link

leopku commented Sep 20, 2022

Got it.
Thank you @efectn .

@piyongcai
Copy link

piyongcai commented Oct 10, 2022 •
edited

@efectn @leopku

How to deal with after merging?embed.FS

embed.FS is compatible with fs.FS (io/fs) interfaces. So it can be used without extra effort. So it'll be easier to use fs interfaces with #2027

Fasthttp FS is not well with fs interfaces but it may be improved in the future. valyala/fasthttp#1374 Perhaps we can remove filesystem middleware in v3 cuz of there will no pros over fasthttp FS after this improvement.

how to suppport Compress option?
use app.Static method, you can set Compress option.

app.Static("/", "web", fiber.Static{
	Compress:  true,     // <---- HERE
	ByteRange: true,
})

@efectn
Copy link
Member Author

efectn commented Oct 10, 2022

@efectn @leopku

How to deal with after merging?embed.FS

embed.FS is compatible with fs.FS (io/fs) interfaces. So it can be used without extra effort. So it'll be easier to use fs interfaces with #2027
Fasthttp FS is not well with fs interfaces but it may be improved in the future. valyala/fasthttp#1374 Perhaps we can remove filesystem middleware in v3 cuz of there will no pros over fasthttp FS after this improvement.

how to suppport Compress option? use app.Static method, you can set Compress option.

app.Static("/", "web", fiber.Static{
	Compress:  true,     // <---- HERE
	ByteRange: true,
})

Probably we will remove filesystem middleware after fasthttp FS gets fs.FS support. (For v3) valyala/fasthttp#1374

Don't have much knowledge about PR at the moment. Not reviewed yet

@efectn efectn added this to the v3 milestone Oct 10, 2022
@piyongcai
Copy link

The implementation of this patch is far off！

@nickajacks1
Copy link
Member

nickajacks1 commented Dec 1, 2023 •
edited

Update, fs.FS support seems to be in fasthttp now.
valyala/fasthttp#1640

@efectn
Copy link
Member Author

efectn commented Dec 2, 2023

Update, fs.FS support seems to be in fasthttp now. valyala/fasthttp#1640

Will work on it

@efectn efectn self-assigned this Dec 2, 2023
@coderabbitai coderabbitai bot mentioned this issue Mar 22, 2024
Merged
@efectn efectn mentioned this issue May 18, 2024
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@efectn efectn

Labels
👍 Accepting PR 🧹 Updates v3
Projects
v3
Status: Done
Milestone
v3
Development

Successfully merging a pull request may close this issue.

middleware: add static middleware gofiber/fiber
4 participants
@piyongcai @leopku @efectn @nickajacks1