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

Improvements #2

Closed
thewhitetulip opened this Issue Feb 27, 2016 · 19 comments

Comments

Projects
None yet
4 participants
@thewhitetulip

thewhitetulip commented Feb 27, 2016

  1. Partial templates are better off as _header.html and _footer.html
  2. Not serving templates on our fileserver along with static files is simple, keep two folders as you have kept in your repo, one for static and one for templates. Pass the static folder to your fileserver
  3. rather than serving static files as a NotFound thing, standardize stuff like if a URL comes like /static/ then it serves the static files
    check the app I built :-)
    http://github.com/thewhitetulip/Tasks/
@jmcfarlane

This comment has been minimized.

Owner

jmcfarlane commented Feb 28, 2016

Thanks for the feedback @thewhitetulip :)

Partial templates are better off as _header.html and _footer.html

What's the rationale for an underscore prefix on template partials?

Pass the static folder to your fileserver

How are you thinking that would work (remembering the goal of a single static binary)? The only idea I've had is to generate (via go-bindata-assetfs) the assets in two different packages using the -pkg flag. Then you could reference pkgStatic.assetFS() with http.FileServer and pkgTemplates.Asset(path) in init().

serving static files as a NotFound thing, standardize stuff like if a URL comes like /static/

This might be a difference imposed by use of httprouter, not really sure.

Thoughts?

@0x434d53

This comment has been minimized.

0x434d53 commented Feb 28, 2016

For exactly your direct exposing problem i wrote a simple FileSystem Wrapper, that only exposes a subdirectory:
https://github.com/0x434D53/WrappedFS

Just see the example.

@jmcfarlane

This comment has been minimized.

Owner

jmcfarlane commented Feb 28, 2016

@0x434d53 happen to know if that would work with github.com/elazarl/go-bindata-assetfs as well as github.com/rakyll/statik?

@jmcfarlane

This comment has been minimized.

Owner

jmcfarlane commented Feb 28, 2016

@julienschmidt happen to know if https://github.com/jmcfarlane/golang-templates-example/blob/master/main.go#L56 is or isn't the intended mechanism with httprouter to serve static assets? (in reference to the comment ^^)?

@0x434d53

This comment has been minimized.

0x434d53 commented Feb 28, 2016

http.FileServer just takes an http.FileSystem. http.FileSystem itself is an interface with just an Open method. This is wrapped by WrappedFS. So it should work with everything that fullfills http.FileSystem - which is true for assetfs. That what's so nice about go's lightweight interfaces.

@jmcfarlane

This comment has been minimized.

Owner

jmcfarlane commented Feb 28, 2016

Slick :)

On Sun, Feb 28, 2016, 10:30 Christoph Seufert notifications@github.com
wrote:

http.FileServer just takes an http.FileSystem. http.FileSystem itself is
an interface with just an Open method. This is wrapped by WrappedFS. So it
should work with everything that fullfills http.FileSystem - which is true
for assetfs. That what's so nice about go's lightweight interfaces.


Reply to this email directly or view it on GitHub
#2 (comment)
.

@julienschmidt

This comment has been minimized.

julienschmidt commented Feb 28, 2016

The clean approach would be to use an assets subdir, but many people seem to use the NotFound handler for it.

The /assets/ / /static/ dir is definitely the way I would recommend.

@thewhitetulip

This comment has been minimized.

thewhitetulip commented Feb 29, 2016

What's the rationale for an underscore prefix on template partials?

Naming templates like that gives you a direct distinguishing factor with the name itself, suppose you have a home.html and account.html and a _head.html then you can know via visual inspection that the head html is not a full fledged template but say a helper template.

@0x434d53 Well we can expose just a directory using the standard http.FileServer by giving it the folder which we want to serve

I agree with @julienschmidt that /static/ or /assets/ is a good way to handle assets also I am not a huge fan of NotFound handler, also I feel static files should not be included inside binaries

How are you thinking that would work

Please do not include everything inside the application, we are building webapps and not console apps, which is why we shuld have the binary and the supporting static files different from each other, it is way more easier to manage code that way.

@jmcfarlane

This comment has been minimized.

Owner

jmcfarlane commented Feb 29, 2016

Please do not include everything inside the application, we are building webapps and not console apps

Very interesting! I think I read a post from Russ Cox who essentially said the same (keep static content outside the binary), yet there is for sure "ease of use" benefits in including them (assuming we're talking a reasonable amount of content). You mention webapp vs console, I'm curious what distinction that has. Is it that a console app is just executed, but a webapp is configured often times in concert with something like Nginx that could handle the static content?

@thewhitetulip

This comment has been minimized.

thewhitetulip commented Feb 29, 2016

Well, console apps do not cater to multiple people :-)

We based apps can cater to million or more people at the same times, that is why it is better for having static files outside the binary.

I have learned quite early as a programmer that the design you choose at the start always sticks till the end, and you'll forever be patching things! It is better to choose a good design at the start so patching isn't required :)

@jmcfarlane

This comment has been minimized.

Owner

jmcfarlane commented Feb 29, 2016

I apologize but I'm still not understanding the relationship between the number of people an application caters to, and how that influences if static content should be included in the binary or consumed at runtime by the binary (from disk). Potentially a specific example might help me understand why to avoid it (I believe you, I just don't understand yet :)

jmcfarlane added a commit that referenced this issue Feb 29, 2016

Improve separation between templates and static content
- Move static content into "static" dir instead of "www".
- Stop using `router.NotFound` in favor of `router.ServeFiles` with a
  named prefix. This constrains the access to assets that start with
  "/static/", hence accomplishing one of the goals.

Relates to #2
@jmcfarlane

This comment has been minimized.

Owner

jmcfarlane commented Feb 29, 2016

Thoughts on ^?

@0x434d53

This comment has been minimized.

0x434d53 commented Feb 29, 2016

@thewhitetulip No we can't, since he is not using http.Dir, but assetFS from go-bindata.

@thewhitetulip

This comment has been minimized.

thewhitetulip commented Feb 29, 2016

Well, according to his problem statement he should be using http.Dir :)
rather than go-bindata

@thewhitetulip

This comment has been minimized.

thewhitetulip commented Feb 29, 2016

@jmcfarlane I am sorry if that came out wrong, it doesn't matter if the million users are being catered to or only one is catered to.

But my point was that it is better to demarcate between binaries and static files

@0x434d53

This comment has been minimized.

0x434d53 commented Mar 1, 2016

@thewhitetulip Well Nr. 8 of his goals is to ship it as one single static binary.

@thewhitetulip

This comment has been minimized.

thewhitetulip commented Mar 1, 2016

I know that there is a lot of hype about the single static binary, but whenever I think of a binary then I tend to think of
binary: to hold your business logic
assets: to store your view logic

I don't think it is a good idea to mix both to put in one binary to rule them all.

As we are building a webapplication and it doesn't make sense to put literally everything into one binary, one reason I have heard is that it'll be easy to run it via any folder, but that isn't how web applications are supposed to be run, that is how desktop applications should be run.

web applications should be deployed, not executed like we execute ls :-)

thewhitetulip/Tasks#17

@jmcfarlane

This comment has been minimized.

Owner

jmcfarlane commented Mar 2, 2016

@thewhitetulip Thanks for the feedback. Your points are valid, though the purpose behind this example repository is to demonstrate at least one way a person could construct a web application that meets stated goals, one of which being a single static binary. A non goal of this project is to convince people that they should be using fully static binaries, but if someone happens to be interested in how - this provides a simple example of how to do it :)

Unless anyone else has comments I'll merge the #3 pull request which addresses a few of the recommendations mentioned in this issue. Feel free to speak up if anything else should be done. Currently I'm not yet making the underscore prefix change as I have not heard sufficient/compelling rationale. If anyone happens to agree that it's a good idea... speak up :)

@thewhitetulip

This comment has been minimized.

thewhitetulip commented Mar 2, 2016

A non goal of this project is to convince people that they should be using fully static binaries

I hope I didn't get you wrong

We must not be advocating full binaries, assets are better off in their own directory

@jmcfarlane jmcfarlane closed this Mar 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment