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

Port to pyramid #115

Open
magical opened this issue Apr 21, 2018 · 23 comments
Open

Port to pyramid #115

magical opened this issue Apr 21, 2018 · 23 comments

Comments

@magical
Copy link
Member

magical commented Apr 21, 2018

This is a meta issue to track plans and progress for porting spline-pokedex to pyramid.

Spline

Spline-pokedex is currently built as a spline plugin. My goal is to convert it into a standalone web app.
I don't want to port spline to pyramid (that's veekun/spline#34). Besides, eevee already did that https://github.com/eevee/spline/

What does spline actually provide? Most of the code can be divided into two broad categories:

  1. plugin support. finding plugins; integrating with pylons; setting up mako template paths; hooks to allow plugins to override things. we don't need any of this.

  2. helper code. useful helpers for web apps. basic index and error pages, basic styling, flashes, wtforms helpers, markdown support, sql logging. we'll probably want to copy some of this into spline-pokedex.

Question: do we want to merge the spline history into this repo?

Veekun

The veekun repo houses "bits and pieces" of the site. Notably, all the styling that makes veekun look like veekun, and some content pages. It gets included in spline-pokedex via some hacky local-plugin mechanism in spline.

I definitely want to merge the styles and header/footer bits into spline-pokedex. Some of the content, like the dex-history, could arguably live elsewhere, but, eh, might as well drag it in.

GTS

The GTS plugin is old and broken, and will likely never be fixed (#72). We should probably just delete it.

Mako

Pyramid has support for mako. Thank god.

Frontpage

Veekun's front page is powered by the spline-frontpage plugin. Ideally i'd like to keep that working, but i'm not sure if it's worth the effort to port, or if it should just be rewritten. This also ties into the question of whether to merge with spline or not.

i18n

One of the things spline provides (with help from Pylons) is internationalization support. I don't think it will be too hard to adapt to using pyramid's i18n support. Not having to deal with plugins should make things vastly simpler

Routing

Pyramid and Pylons have different routing systems. Pylons uses routes, which is based around a controller/action model. Pyramid has its own system which is built around views

Pyramid also has a traversal-based routing system that you can use, which would probably be a good fit for the pokedex... but it's also different from what we're doing now, and would be more effort to switch to, so for now I intend to stick with the old pattern-based routing system.

Views

The biggest difference between pyramid and Pylons is probably the view system. Also, pyramid doesn't use threadlocal objects for things like the request and template context, unlike Pylons, which uses them all over the place. So we'll have to make sure that we're threading the context through explicitly to every function that needs it. This is probably what will cause the most strife during porting.

@magical
Copy link
Member Author

magical commented Apr 22, 2018

Here's my overall plan for doing the port. There are actually several steps that we can do while we're still using spline.

Prep work

  • delete the gts plugin (GTS is completely broken #72)

  • merge veekun into spline-pokedex

  • remove or simplify the two uses of spline.lib.flash. (this isn't strictly necessary, but i'd like to get rid of all uses of pylons.session)

  • change obdurate to not use pylons.session

  • remove all uses of pylons.tmpl_context (c) and pylons.url that occur outside a controller or template. many of the functions in pokedex.helpers will need to be moved to lib.mako.

  • copy spline static assets and templates into spline-pokedex

  • get rid of "widgets" by moving them into the templates they are supposed to be included in

  • copy helpers from spline.lib.helpers into pokedex.helpers (there are only a few)

  • copy spline.lib.forms into spline-pokedex

Pyramid port begins

  • copy pyramidapp.py from my current wip. this sets up routes and machinery for the pyramid app.
    it injects h & c variables into the template context like pylons.
    it will also inject a url shim that simulates the pylons.url api.
    we'll be using a dummy translator for starters.

  • convert controllers into views. this will be kind of a slog, but it should be fairly straightforward after all the previous changes

  • update all references to h.pokedex to h in templates

  • update all url calls to use the new routing names, remove compatibility shim

  • remove all the from spline import i18n lines from templates
    (they won't be necessary)

  • recreate the frontpage plugin

  • convert tests. i have no idea how testing works in pyramid

  • figure out caching

  • rename splinext.pokedex to something more suitable

  • figure out i18n

@magical
Copy link
Member Author

magical commented Apr 22, 2018

I ported the lookup page to pyramid as a prototype. I think i should also try one of the smaller dex pages (maybe abilities) before doing this for real.

I turns out we don't need much of spline, so i don't think it's worth merging the history into spline-pokedex.

@magical
Copy link
Member Author

magical commented Apr 27, 2018

I ported the abilities page today (5e9d014). Was pretty straightforward. Required minimal changes to the controller and template. The main difficulty was updating references to the helper functions that i moved, and fixing up one that i had broken during the move. (47ff94f)

magical added a commit that referenced this issue Apr 27, 2018
Instead of implicitly stashing c.seen on the template context,
explicitly stash it on a class.

This is mainly to avoid one of the two uses of c in h.pokedex,
which will help during the move to pyramid.

Updates #115.
magical added a commit that referenced this issue Apr 28, 2018
Pyramid requires access to the request object to be able to generate
URLs, so move helpers that generate URLs to dexlib, where they'll have
access to the template context. This seems easier than adding an
explicit 'request' or 'url' parameter to all of them.

The next commit will fix up calls to the old functions.

- The _=_ parameters are no longer necessary because we already have
  access to the translator. Keep the parameters so that we don't break
  callers, but change the default to _=None because there is no longer
  a global _ variable in scope.

Updates #115.
Updates #117.
magical added a commit that referenced this issue Apr 28, 2018
Needs access to dexlib.pokemon_form_image to generate pokemon icons.
Rather than moving it to dexlib, introduce a callback for getting the
icon.

Updates #115
@magical
Copy link
Member Author

magical commented Jun 10, 2018

I intend to pick this up again sometime in the next few weeks.

@eevee, any thoughts?

@eevee
Copy link
Member

eevee commented Jun 19, 2018

I think this would be fantastic, and of course more immediately helpful than veekun-pokedex which is a long ways from being anything. Maybe it'd also give me an excuse to run through and spruce up basically everything one bit at a time.

veekun-pokedex does have a couple niceties: the move table (so far) is built from objects and is much less haphazard in general, and the routing is done via resources which is kind of cool but may or may not be more trouble than its worth. Also it has language right in the URL, which I understand to be the Right Way to go about that.

@eevee
Copy link
Member

eevee commented Jun 19, 2018

Looking through the pages you ported and I'm pleasantly surprised with how straightforward it is. I guess with the template and database libs the same, the only changes are at boundaries, where we really don't do a lot.

I suppose we should get rid of the c references eventually, and I want to say there's something much simpler we could be doing with SQLA with regards to eagerloading, though now offhand I can't remember what it was. I think around 1.1 they added a way to do explicitly something we've been doing implicitly? Separate query by id, maybe? We should definitely be more consistent about it; so far I've always just looked at the queries and tacked on eagerloads until the overall count becomes reasonable.

(Or there's always the YAML thing, which someday might actually exist.)

Pyramid does have a lot more in the way of plugin APIs compared to Pylons, so I suspect caching won't be too hard.

And, yeah, forget spline. Failed experiment. New spline is barely even the same thing and effectively only powers one website anyway, again.

@magical
Copy link
Member Author

magical commented Jun 19, 2018

veekun-pokedex does have a couple niceties: [...]

It does. Once the initial port is complete we can start pulling things piecemeal from veekun-pokedex.

I suppose we should get rid of the c references eventually

Definitely.

@magical
Copy link
Member Author

magical commented Jun 19, 2018

Are you fine with merging the veekun and spline-pokedex repos?

@eevee
Copy link
Member

eevee commented Jun 19, 2018

But... but what if someone wants to run exactly my same website with different branding?!

(yeah go ahead, this spline will be dead after the port anyway, might as well break a few eggs)

@magical
Copy link
Member Author

magical commented Jul 13, 2018

Users/Forum

I guess i forgot mention this in the first post, but spline also provides the users and forum plugins. I don't want to reimplement those, so my intention is to let them go away. The forum is dead anyway, and the login system is based on WebFinger/BrowserID, so i doubt anyone can even log in any more.

I did grab an archive of all the posts with wget, for whatever that's worth.

magical added a commit to magical/spline-pokedex that referenced this issue Oct 30, 2018
@magical
Copy link
Member Author

magical commented Oct 30, 2018

So, i kind of made this more stressful than it needed to be by trying to separate things cleanly into "pre-merge" and "post-merge" steps. The problem is that — as i discovered during my first prototype attempt — there are lots of unexpected issues that can arise during the porting process. Things that could be addressed pre-merge (if i knew about them) but i can't discover until i'm deep in post-merge. And i don't know what they are because i haven't completed a full port yet.

The new plan is to just create a branch, do the merge with veekun, and try to complete as much of the port as possible. Then after everything is working, i'll see if i can factor it into a more reasonable series of commits (or maybe just not bother and merge it as-is). If i find anything that i can cherry-pick to the master branch, i'll probably do that on a case-by-case basis as i go.

That's happening here: https://github.com/magical/spline-pokedex/tree/pyramid_pokedex

@magical
Copy link
Member Author

magical commented Oct 31, 2018

I'm tracking my progress using the Projects tab, which is actually fairly slick: https://github.com/veekun/spline-pokedex/projects/1

magical added a commit to magical/spline-pokedex that referenced this issue Nov 3, 2018
Needs access to dexlib.pokemon_form_image to generate pokemon icons.
Rather than moving it to dexlib, introduce a callback for getting the
icon.

Updates veekun#115
@magical
Copy link
Member Author

magical commented Nov 4, 2018

All pages have been ported (except error pages). Tests have mostly been ported.

magical added a commit to magical/spline-pokedex that referenced this issue Jul 14, 2019
Gonna have to pull beaker in for caching anyway, so might as well use it
for sessions. I'd rather do without sessions, but i don't want to go
through the trouble of figuring out something different right now.

While we're at it, set the minimum version for pyramid_beaker to 0.8 (the
latest release, from 2013).

Updates veekun#115
@magical
Copy link
Member Author

magical commented Jul 15, 2019

I'm picking this up again. The plan is to continue to plug away at the pyramid_pokedex branch until it reaches feature parity with the current version. Meanwhile, i'll backport as much as I can to master.

magical added a commit that referenced this issue Jul 20, 2019
magical added a commit that referenced this issue Jul 20, 2019
magical added a commit that referenced this issue Jul 20, 2019
Pyramid requires access to the request object to be able to generate
URLs, so the plan is to move helpers that generate URLs to dexlib, where
they'll have access to the template context and thus the request object.
This seems easier than adding an explicit 'request' or 'url' parameter
to all of them.

Copy image and url helpers from splinext.pokedex.helpers to dexlib.
Update any internal references to those helpers, but leave external
callers alone for now. The next few commits will start fixing up calls
to the old functions.

Keep the old functions around (for now) so as not to break anything.
We can't do a move+update in one commit anyway because there is code in
the `veekun` repository which still uses the old helpers.

The _=_ parameters are no longer necessary because template functions
already have access to the translator. Keep the parameters so that we
don't break callers, but change the default to _=None because there is
no longer a global _ variable in scope. (The default value doesn't
matter - it gets immediately overwritten by a line that gets injected
into template functions, `_ = i18n.get_translator(lambda i18n, c)`)

Updates #115.
Updates #117.
magical added a commit that referenced this issue Jul 20, 2019
Needs access to dexlib.pokemon_form_image to generate pokemon icons.
Rather than moving it to dexlib, introduce a callback for getting the
icon.

Updates #115
Updates #117
@magical
Copy link
Member Author

magical commented Jul 20, 2019

Created the pyramid-backports branch: https://github.com/veekun/spline-pokedex/compare/pyramid-backports. It contains changes that can be safely backported to the spline-based code. The goal here is to minimize the difference between the current code and the eventual pyramid code.

@magical
Copy link
Member Author

magical commented Jul 22, 2019

Feels like i'm getting close!

@magical
Copy link
Member Author

magical commented Aug 6, 2020

I want to finish this up. As of the last time i touched it (in mid 2019) it was pretty much complete. I'm sure there are still some unpolished areas, and there are some open questions, but nothing major imo.

I think the next step that needs to happen is to deploy a test instance. Then we can evaluate it and decide how to proceed.

Open questions:

  • write a new frontpage or port the old one?
  • figure out what to do with icons. spline contains complete copies of the famfamfam flags and Fugue icon sets. i'm not sure whether to copy all of them to spline-pokedex, copy just the ones needed, or something else. (n.b. this doesn't really matter)
  • pyramid has no built-in way to send email reports of crashes. we'll want to find some way to replace that feature
  • should we actually merge into spline-pokedex, or leave this repo as a historical record and create a new repo?

@eevee
Copy link
Member

eevee commented Aug 13, 2020

write a new frontpage or port the old one?

The old one is... not... great? The Bulbanews feed doesn't seem to work any more, my blog isn't especially relevant, and the changelog dates back to gitweb days (whereas now GitHub is much easier to read, if you care to). The only other bit of interest is the forums, which no one uses.

It could probably stand to be full of stuff that's actually relevant to the site — a big friendly index (rather than forcing new users to peer through menus), a bit more about the project itself and its status, contributors and how to find them, link to the Discord, etc.

figure out what to do with icons. spline contains complete copies of the famfamfam flags and Fugue icon sets. i'm not sure whether to copy all of them to spline-pokedex, copy just the ones needed, or something else. (n.b. this doesn't really matter)

Could... we... just use flag emoji, now?

Offhand I'm not sure how much of fugue we're actually using, outside of the forums.

pyramid has no built-in way to send email reports of crashes. we'll want to find some way to replace that feature

Probably not that service I accidentally spent over a grand on after they quietly jacked up the price to like $60 a month.

I see a pyramid_errmail but it's quite a bit old. Kinda surprised I don't see anything obvious for doing this. Or maybe people noticed that receiving tens of thousands of emails is not the ideal way to deal with errors.

Could log them locally, and write a thing to send them to Discord...?

should we actually merge into spline-pokedex, or leave this repo as a historical record and create a new repo?

If this is a single consolidated codebase without all the plugin nonsense, then I'd say create a new one. This mess is confusing enough. :) It's what I was doing with my own Pyramid from-scratch experiment (veekun-pokedex), too. We can figure out how to migrate issues later.

@magical
Copy link
Member Author

magical commented Aug 13, 2020

Definitely agree that the frontage needs a redesign, but since @hugopeixoto just ported the old one, this is no longer standing in the way of the port 🎉

P.S. afaict Bulbanews still works, they just... haven't... posted anything in a while. Saw something on Twitter that they were looking for new writers.

@magical
Copy link
Member Author

magical commented Aug 13, 2020

Could... we... just use flag emoji, now?

Oh. Possibly. The icons are probably slightly nicer for screenreaders, since we can give them useful alt text.

Offhand I'm not sure how much of fugue we're actually using, outside of the forums.

We don't use much of it, but there are definitely a few sprinkled around the site. I don't really want to audit every file to track down which ones we're using though. (The frontpage, for example, uses icons which are specified in the ini file)

Pyramid has a nifty feature where you can serve files from other eggs. I was thinking it might be nice to throw a famfamfam-icons project up on pypi and use that. Although that would probably complicate serving them from nginx in production.

@magical
Copy link
Member Author

magical commented Aug 14, 2020

I see a pyramid_errmail but it's quite a bit old.

No commits since 2015, hmm. I do see that its dependency, pyramid_mailer was touched somewhat more recently. Might be a case of "it's stable, why would we change anything?"

@eevee
Copy link
Member

eevee commented Aug 16, 2020

Tada: https://beta.veekun.com/

Freshly provisioned Linode, even! Gonna gradually migrate stuff over because I love inventing work for myself.

@magical
Copy link
Member Author

magical commented Mar 9, 2021

should we actually merge into spline-pokedex, or leave this repo as a historical record and create a new repo?

proposal:

  1. fork spline-pokedex to zzz-spline pokedex
  2. merge pyramid branch into spline-pokedex
  3. rename spline-pokedex to pyramid_pokedex

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

No branches or pull requests

2 participants