#pyramid

/

      • otter768 joined the channel
      • joules joined the channel
      • zepolen joined the channel
      • hvelarde has quit
      • zepolen has quit
      • eatkin has quit
      • otter768 has quit
      • eatkin joined the channel
      • otter768 joined the channel
      • erasmas has quit
      • mejymejy joined the channel
      • goodwill
        sontek1: ping
      • mejymejy has quit
      • Method__
        pong
      • Arfrever joined the channel
      • conan_the_destro joined the channel
      • kamalgill has quit
      • cewing has quit
      • tiwula has quit
      • mejymejy joined the channel
      • mejymejy has quit
      • zepolen joined the channel
      • zepolen has quit
      • joulez joined the channel
      • blaflamme has quit
      • joules has quit
      • mejymejy joined the channel
      • rickmak joined the channel
      • rickmak__ joined the channel
      • rickmak has quit
      • rickmak__ is now known as rickmak
      • raydeo
        I'm like 40% of the way to dropping cache buster support in 1.6 until we can figure out a workable solution...
      • because I want to release 1.6
      • dstufft: we should chat quickly about this stuff when you have a sec
      • dstufft
        raydeo: whats up
      • oh :(
      • joulez has quit
      • raydeo
        x58 and I were going over options for a config.add_cache_buster(spec, buster) api
      • the core issue is that for matching there isn't a deterministic way to figure out which buster a busted url should be unbusted with
      • if there are more than one buster for a static view prefix (/static/...)
      • dstufft
        this is for the other method that manifest and such doesn't use?
      • raydeo
        which would happen if you called add_cache_buster(..) with 2 specs that hosted files on a single static view
      • yeah... matching side is the match method
      • our only option with that api is to call match on all busters until one returns something
      • x58 added a summary of the issue to https://github.com/Pylons/pyramid/issues/2145
      • driti joined the channel
      • dstufft
        A possible middle ground that still lets you get most of the benefit of cache busting and side steps the problem is to just remove the match() API in 1.6? You can always add it back in later but in 1.6 you can say that pregenerate must create an URL that still resolves to a servable file (so it either needs to create a filename that exists, or i tneeds to use query strings)
      • raydeo
        a search/loop isn't actually that bad to me but it does ruffle my features a bit
      • feathers*
      • yeah in my own personal apps the url does not map to a file on disk :(
      • rickmak has quit
      • not a bad idea though, I'd consider it
      • dstufft
        raydeo: really? Every pipeline I've ever used computed a hash and then dropped it on disk. So that's a new one too me
      • that wouldn't be super fun, though I think you could restore it by using a custom static view?
      • I think I feel similarly that a search/loop isn't super bad, but feels a bit dirty, but I don't have a better answer
      • raydeo
        I'm not in love the current match api either, so removing it isn't a bad option
      • match(subpath) I think may not be quite enough parameters for all scenarios.. like I'm not sure why we wouldn't pass request, or at least query string as well
      • dstufft
        It feels like to me that the pregenerate function is enough to get a 95% solution to the problem but I may be the outlier instead of the common case :D
      • an easy 95% solution that doesn't block additional work to add the final 5% seems like a pretty good trade off to me
      • raydeo
        right, I also feel like it is pretty good
      • you could also solve it with a tween/decorator/etc
      • also static views can be externally hosted so pyramid may never even see the requests... another reason to drop it from the api
      • dstufft
        solve the match() side you mean?
      • raydeo
        right
      • dstufft
        yea
      • raydeo
        I think you've convinced me... gonna add it to the ticket and see what x58 says
      • I don't think he actually cares about cache busting so he'll probably be happy to drop it
      • dstufft
        ha
      • raydeo: maybe add a note to the documentation or the cookbook about how you reverse the pregenerate() function using a tween or a custom static view or whatever
      • how you can*
      • raydeo
        I actually have this solved in my own apps like this http://paste.ofcode.org/QADj6Lfq3jyHkYgbju2Mtf
      • very simple
      • joules joined the channel
      • dstufft
        raydeo: copy/paste that into cookbook :D
      • raydeo
        oh hey... my other todo is https://github.com/Pylons/pyramid_tm/issues/40 if you have a second to think about how it might affect warehouse
      • dstufft
        raydeo: I'm not sure your timelines here for 1.6, but if it's in the next 2 weeks or so Maybe 3 or 4, we'll see!) my availability is going to be all over the place and twitter/email is likely going to be the best way to get ahold of me fwiw.
      • dstufft looks
      • raydeo
        sure, no problem
      • rickmak joined the channel
      • if we do this cache buster fix I think we could have final out in a couple weeks max
      • dstufft
        I'm closing on my house on the 4th and we have movers scheduled for the 11th, so it's gonna be crazy
      • travisfischer has quit
      • raydeo
        exciting!
      • dstufft
        None of our tweens touch the database other than pyramid_tm and maybe one I have for Sentry/Raven integration, but that's read only so I don't think it can trigger any of the problems described in this ticket. Trying to think the implications through, we do touch the DB in the exception view if it's a forbidden exception (so we can redirect if the user isn't logged in) but I don't think that would trigger this error either.
      • I *think* if I understand the ticket and the suggestion right, the split would make this better right now, I guess the forbidden view might technically trigger a second transaction
      • it seems reasonable to me though
      • raydeo
        yeah atm transactions are over prior to exception views
      • ricoalpha joined the channel
      • if you are creating a new one it may never be getting closed
      • could be an issue
      • dstufft
        oh!
      • that probably explains
      • raydeo
        an alternative approach would be to close the transaction in request.add_finished_callback() instead actually, instead of an over tween
      • mejymejy has quit
      • dstufft
        I had to do that, and I thought pyramid_tm was supposed to handle it but I hadn't gotten around to figuring out why
      • raydeo
        would encompass other things besides tweens
      • ah, yeah
      • so this fix would solve that
      • dstufft
        might still be reasonable to split up the tween, one that opens the transaction and adds the request.add_finished_callback and one that handles retries to still give that control so that you get coverage over things outside of tweens too, make sure to clean up the connection, and the higher the over tween is in the stack, it could easily be the last registered callback as long as it was added after the inner tweens/hnadler had been called
      • ricoalpha has quit
      • raydeo
        yeah the other subtle issue is that request.add_response_callback() shouldn't touch the database
      • dstufft
        I should probably make sure I'm not touching the DB there
      • raydeo
        so instead of an over-tween I could just add a finished_callback which would commit/abort at the very end
      • this is hairy as hell though
      • if I change the behavior it means you can't catch database errors in an exception view if you wanted to
      • unless you call db.flush() or something
      • under the excview
      • I hadn't thought of that... could be a bigger bw-compat issue than I was initially thinking
      • atm transaction.commit() failures can be caught in excviews
      • dstufft
        oh, yea :(
      • that would turn integrity failures into outright errors instead of catchable errors
      • raydeo
        yeah, unless it's retryable
      • rickmak has quit
      • in which case the under would retry it
      • dstufft
        raydeo: well though, if you split it, they could still configure it so that the two tweens are alongside each other
      • raydeo
        yeah I see now why this is such an issue to move the commit over excview.. grumble
      • x58
        Ive been thinking of creating a new transaction in my exception views.
      • raydeo
        it's a real issue in my apps, and apparently yours as well, that you can't use anything in exception views
      • rickmak joined the channel
      • x58
        Old stuff can't be used, but that is a minor issue for me.
      • raydeo
        x58: that seems to be the only option... the problem is all the cached request methods
      • (which is an issue for me)
      • you can only easily do a new transaction if you replace the excview_tween in pyramid anyway
      • and doesn't solve the issue I mentioned with NewResponse events and request.add_response_callback
      • x58
        I just got back to my computer, haven't read scrollback yet.
      • rickmak has quit
      • rickmak joined the channel
      • raydeo: For the cache busting I'd instead change the API so that the cache buster can only mess with the last part of the path.
      • raydeo
        just mess with the filename and query string you mean
      • x58
        Also, in case there are multiples, when adding a cache buster, we know what it is for, lookup the override, and then look up what it is overriding, and somehow let the static view know that there is a new cachebuster.
      • So even if we have to loop, it's a limited set.
      • Yes.
      • travisfischer joined the channel
      • raydeo
        I'm not sure only busting the filename is acceptable
      • at the very least the buster needs the full path, whether it busts only the last segment or not
      • maduro joined the channel
      • x58
        I don't mind giving it the full path, but it should only modify the last part of it (filename)
      • dstufft
        limiting it to only being able to bust the filename just to handle matching seems like a worse option than just killing matching (at least ofr now)
      • raydeo
        that's a subset of what the manifest format allows (mapping path to path), I'm -1 on that I think
      • x58
        It's just an idea.
      • Removing the matching for now doesn't seem worth it.
      • raydeo
        yeah it'd make the impl easier for sure
      • what do you mean it doesn't seem worth it?
      • x58
        We still have the issue, and now we have an API we've shipped and need to worry about b/w for.
      • I was replying to dstufft's suggestion. It feels half-baked to me.
      • raydeo
        I updated #2145 with the removal idea
      • x58
        I'll do it begrudgingly just to get a 1.6 out the door.
      • raydeo
        what's half-baked about it?