versions in background?

[jroc...@sciencehistory.org]

The versions plugin: https://shrinerb.com/rdoc/classes/Shrine/Plugins/Versions.html

Suggests you can process versions in the background: "If versions are processed in a background job…"

And even suggests you can process _some_ versions in the foreground and others in the bg: "If you already have some versions processed in the foreground after a background job is kicked off (with the recache plugin)"

I don't entirely underestand what approach it's suggesting to do both these things.  Is it just by using the `backgrounding` plugin to make the _entire_ process of promotion (including versions) be in the bg?

Are there any code examples anywhere of both these things?  Versions in bg, and some versions in bg and some in fg. I don't entirely understand how the `recache` plugin works. 

Thanks for any tips! 

Jonathan

[Hiren Mistry]

I think the docs are clear without overloading the user in tons of code samples.

To process in foreground, use the recache plugin. It's like the processing plugin but before promotion in a before_save callback in the web process so you want to only do quick stuff. See https://shrinerb.com/rdoc/classes/Shrine/Plugins/Recache.html

Which part do you not understand? If you can take a moment to highlight what's confusing you and see how could the docs explain it better so other new users can follow.

To process in foreground and background, use the recache, processing, and backgrounding plugins. Recache will process in foreground in the web process, processing will do the more intensive stuff in the background because of the backgrounding plugin. If you weren't using the backgrounding plugin, then you may not need the recache plugin because processing plugin will happen in the foreground.

We have templates for creating simple code to play with (that's what I do to test out things). See https://github.com/shrinerb/shrine/blob/master/CONTRIBUTING.md 

[Rochkind, Jonathan]

I’ll try to figure it out. I think the part I don’t understand is just how to write the code, heh. I still think a simple code example of something processing versions in both foreground and background would be helpful. I think it’s easy to lose track of how mystifying shrine’s abstraction can be when you’re getting started.

 

__________________________________

Jonathan Rochkind
Software Developer/Technical Lead
Othmer Library of Chemical History
t. +1.215.873.8239

Science History Institute 
Chemistry • Engineering • Life Sciences 
315 Chestnut Street • Philadelphia, PA 19106 • U.S.A.
sciencehistory.org

The Chemical Heritage Foundation is now the Science History Institute. For more information visit sciencehistory.org.

--
You received this message because you are subscribed to a topic in the Google Groups "Shrine" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/ruby-shrine/_A2BNmbxa-g/unsubscribe.
To unsubscribe from this group and all its topics, send an email to ruby-shrine...@googlegroups.com.
To post to this group, send email to ruby-...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ruby-shrine/45bf4a63-42f1-4052-aaf5-2e4ee08d3de0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Rochkind, Jonathan]

PS:

 

You say “To process in foreground, use the recache plugin.” – but you don’t actually need the recache plugin to process in the foreground, do you?

 

The basic example in versions does not use the recache plugin, just `processing`,  and presumably processes in the foreground?

 

Maybe you only need the recache plugin when you want to process some versions in the foreground AND others in the background?  So how you do ‘foreground’ changes depending on whether you are also doing background or not?

 

Or do I have this totally wrong?

 

I think the name of the ‘recache’ plugin is confusing I don’t understand what it has to do with caching/recaching.

 

Jonathan

[Rochkind, Jonathan]

OK, sorry, I should have waited and sent all my emails at once, sorry!

 

So, the backgrounding plugin will move the _entire_ promotion operation into a background job, correct?  Which by default will mainly include copying the bytes from cache to store, if that’s required.

 

Do I understand correctly that the suggestion to use this to move version creation into the bg is to indeed move the entire promotion operation into the bg, with version creation being part of that?

 

Or is there a different way to use it so version creation is in the bg, but ordinarily promotion is still in the fg?

 

Jonathan

 

[Hiren Mistry]

see below.

On Friday, September 7, 2018 at 9:53:10 AM UTC-7, Rochkind, Jonathan wrote:

 I think it’s easy to lose track of how mystifying shrine’s abstraction can be when you’re getting started.

Sure, hence me asking you to make notes of how we can improve docs for beginners. We're too familiar to see it now. :)

You say “To process in foreground, use the recache plugin.” – but you don’t actually need the recache plugin to process in the foreground, do you?

 Yes this is what I said in my earlier response.

The basic example in versions does not use the recache plugin, just `processing`,  and presumably processes in the foreground?

 Correct.

Maybe you only need the recache plugin when you want to process some versions in the foreground AND others in the background?  

 Correct again. It's what I explained in my last response. The process(:store) blocks get moved into a background job because of the backgrounding plugin. Remove the latter, and the process happens in the foreground. The recache plugin is only needed to provide you a way to do some foreground processing because you used the backgrounding plugin which moved the process(:store) blocks into bg jobs.

So how you do ‘foreground’ changes depending on whether you are also doing background or not? Or do I have this totally wrong?

Same answer as above. I'll let it sink in.

 

I think the name of the ‘recache’ plugin is confusing I don’t understand what it has to do with caching/recaching.

I can agree with this. I think the name comes from Shrine having to download the file from the storage(:cache), do some processing, and then re-uploading it back to storage(:cache)... hence recache. :)

OK, sorry, I should have waited and sent all my emails at once, sorry!

 

So, the backgrounding plugin will move the _entire_ promotion operation into a background job, correct?  Which by default will mainly include copying the bytes from cache to store, if that’s required.

Yep and invokes any process(:store) blocks in the process after promoting.

 

 

Do I understand correctly that the suggestion to use this to move version creation into the bg is to indeed move the entire promotion operation into the bg, with version creation being part of that?

 

Or is there a different way to use it so version creation is in the bg, but ordinarily promotion is still in the fg?

 

Jonathan

 

Correct. I don't think you can separate promotion in fg and version creation in bg just using the plugins. Maybe you can hack some custom low level code to try and do that if you want.

[Janko Marohnić]

What Hiren said :)

I just wanted to add that the derivates plugin is intended to deprecate the following plugins:

• versions
• processing
• recache
• delete_raw

So I wouldn't recommend thinking too much about how these plugins work, you should be able to skip the corresponding Ruby files when understanding the Shrine codebase.

With the new derivates plugin anything derivate-related will hopefully be explicit, so that should remove any confusion which derivate is processed in fg/bg/other, because the user explicitly calls the processing code, so they can decide whether that will be in the controller, background job, 2nd background job etc.

Kind regards,

Janko

--
You received this message because you are subscribed to the Google Groups "Shrine" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ruby-shrine...@googlegroups.com.

To post to this group, send email to ruby-...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/ruby-shrine/74882de0-d593-4cc0-9dcf-cc8ebb4438fb%40googlegroups.com.

[Rochkind, Jonathan]

Thanks!  Good to clarify it would be intended to deprecate them all.

 

I think it’s possible `processing` should be left there for when/if you want to transform an ‘original’ upon upload, without having verisons/derivatives, just a single thing, but transformed on upload.

 

I’m trying to understand how the existing stuff works as a platform for how ‘derivatives’ might work.

 

I might try to really trace some of the code, and write it up, perhaps in my blog.

 

One thing that I find MIGHT make the existing code a bit harder to understand is – the way the ‘core’ functionality is implemented as a plugin. This is clever, but provides an additional jump I have to make in my head to understand where a given core method is, from the plugin module to the actual core object. I’m still a bit confused about the roles/relationships between “attacher”, “attachment”, “uploader” – and a fourth object is involved I think, the straight `Shrine` class object?  Even though uploader is actually a subclass of Shrine, an operation with an uploader involves a non-subclassed Shrine too I think? 

 

If you wanted to write a couple paragraphs about uploader/attacher/attachment/shrine objects, what each are and their relation to each other, that might be good overview for someone trying to dive into the code. It also might be worth at least considering taking the core func out of a plugin and making ‘ordinary’ attacher/attachment/uploader classes, in separate files, to ease code reading.

 

In considering and trying to get started implementing derivatives, potential race conditions are seeming one of the hardest parts to me at the moment.  It might be very hard to do this in an ORM-agnostic way.  If derivatives are not in the same column as the original, they no longer need to be a part of the `process` step, or even really a hard-coded part of the uploader at all. You could theoretically hook into any point to generate derivatives, and then just call `add_derivative(:something, io)` or something  – or launch your own bg job in after_commit (say if you wanted promotion in foreground but derivs in bg) just fetched the record and then called `add_derivative`. The trick in this later case is race conditions.

 

Incidentally, I believe there is in fact a race condition in the current backgrounding plugin, and no way to get around a race condition without using either optimistic or pessimistic locking. I may file a ticket on that.

 

I’m currently trying setting a goal of derivatives much like your suggest in my GH Issue – two columns, with actual two attachers/attachments, with the one containing the actual derivatives actually still using something much like the `versions` plugin to make it into a hash of things.  I’ve sort of started to code, but more just trying to figure out what’s going on “normally” (so I can avoid breaking things), and figuring out what the options/affordances are.  Still a lot of playing around and changing what I’m doing every few hours, not sure whether or not it would be useful to share.

 

Jonathan

 

__________________________________

Jonathan Rochkind
Software Developer/Technical Lead
Othmer Library of Chemical History
t. +1.215.873.8239

Science History Institute 
Chemistry • Engineering • Life Sciences 
315 Chestnut Street • Philadelphia, PA 19106 • U.S.A.
sciencehistory.org

The Chemical Heritage Foundation is now the Science History Institute. For more information visit sciencehistory.org.

From: <ruby-...@googlegroups.com> on behalf of Janko Marohnić <janko.m...@gmail.com>
Date: Saturday, September 8, 2018 at 6:12 PM
To: Hiren Mistry <hiren....@chai-monsters.com>
Cc: ruby-shrine <ruby-...@googlegroups.com>
Subject: Re: versions in background?

 

What Hiren said :)

--
You received this message because you are subscribed to a topic in the Google Groups "Shrine" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/ruby-shrine/_A2BNmbxa-g/unsubscribe.
To unsubscribe from this group and all its topics, send an email to ruby-shrine...@googlegroups.com.

To post to this group, send email to

ruby-...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ruby-shrine/CAJ8a-RMnpYa1MYoTB%3Db%2BDJtooY%2BSDztOupXzQdVFZ3O%3D7DyWsA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Hiren Mistry]

On Sunday, September 9, 2018 at 9:11:51 AM UTC-7, Rochkind, Jonathan wrote:

Thanks!  Good to clarify it would be intended to deprecate them all.

 

I think it’s possible `processing` should be left there for when/if you want to transform an ‘original’ upon upload, without having verisons/derivatives, just a single thing, but transformed on upload.

 

I’m trying to understand how the existing stuff works as a platform for how ‘derivatives’ might work.

 

I might try to really trace some of the code, and write it up, perhaps in my blog.

 

One thing that I find MIGHT make the existing code a bit harder to understand is – the way the ‘core’ functionality is implemented as a plugin. This is clever, but provides an additional jump I have to make in my head to understand where a given core method is, from the plugin module to the actual core object. I’m still a bit confused about the roles/relationships between “attacher”, “attachment”, “uploader” – and a fourth object is involved I think, the straight `Shrine` class object?  Even though uploader is actually a subclass of Shrine, an operation with an uploader involves a non-subclassed Shrine too I think? 

 

I faced similar issue in reading the code. I don't have an issue with base being a plugin but the roles and relationships of the 4 pieces are intertwined making it harder to say this is doing this and then that will do that. Also the flow doesn't exactly read off clearly.

 

If you wanted to write a couple paragraphs about uploader/attacher/attachment/shrine objects, what each are and their relation to each other, that might be good overview for someone trying to dive into the code. It also might be worth at least considering taking the core func out of a plugin and making ‘ordinary’ attacher/attachment/uploader classes, in separate files, to ease code reading.

Janko has already written this as best he can. What would help now is some one in your shoes (i.e. beginner) saying if this guide explained __something__ in this way, it would be a lot clearer. We are too familiar with it to go back in time and see how a beginner might be lost or understanding it.

See these guides:

https://shrinerb.com/rdoc/files/doc/design_md.html

https://twin.github.io/the-plugin-system-of-sequel-and-roda/

other docs here https://github.com/shrinerb/shrine/tree/master/doc

 

 

In considering and trying to get started implementing derivatives, potential race conditions are seeming one of the hardest parts to me at the moment.  It might be very hard to do this in an ORM-agnostic way.  If derivatives are not in the same column as the original, they no longer need to be a part of the `process` step, or even really a hard-coded part of the uploader at all. You could theoretically hook into any point to generate derivatives, and then just call `add_derivative(:something, io)` or something  – or launch your own bg job in after_commit (say if you wanted promotion in foreground but derivs in bg) just fetched the record and then called `add_derivative`. The trick in this later case is race conditions.

 

Incidentally, I believe there is in fact a race condition in the current backgrounding plugin, and no way to get around a race condition without using either optimistic or pessimistic locking. I may file a ticket on that.

Please do and provide a test script to reproduce the problem. We want to make sure things work very well.

 

 

I’m currently trying setting a goal of derivatives much like your suggest in my GH Issue – two columns, with actual two attachers/attachments, with the one containing the actual derivatives actually still using something much like the `versions` plugin to make it into a hash of things.  I’ve sort of started to code, but more just trying to figure out what’s going on “normally” (so I can avoid breaking things), and figuring out what the options/affordances are.  Still a lot of playing around and changing what I’m doing every few hours, not sure whether or not it would be useful to share.

You can share either here or privately with me to discuss it. It helps when you have to explain something or figure out what your confusion is and how to ask it. :)

[Janko Marohnić]

I think it’s possible `processing` should be left there for when/if you want to transform an ‘original’ upon upload, without having verisons/derivatives, just a single thing, but transformed on upload.

Yeah, that's the use case why the processing plugin and versions are separate plugins. But I would now prefer if people would trigger this processing themselves when they want to, e.g:

resized        = ImageProcessing.source(photo.image).resize_to_limit!(1200, 1200)

stored_resized = photo.image_attacher.store!(resized)

photo.image_attacher.set(stored_resized)

photo.save

It's nice and explicit, you know exactly when it happens, no magic around it (though we should somehow allow merging 2nd and 3rd step into a single command). That also covers the case when people would like to process the original outside of the regular attachment flow, which I recall was requested once or twice.

If you wanted to write a couple paragraphs about uploader/attacher/attachment/shrine objects, what each are and their relation to each other, that might be good overview for someone trying to dive into the code.

I already did, see the Design of Shrine guide :)

It also might be worth at least considering taking the core func out of a plugin and making ‘ordinary’ attacher/attachment/uploader classes, in separate files, to ease code reading.

I would like to do that, but then it wouldn't be possible for plugins to override base functionality. If core methods were defined directly on the classes, then modules included into those classes wouldn't be able to override those methods. Putting the core functionality into modules and including them into core classes as 1st modules allows subsequent modules to override methods from the base modules.

It's an interesting bit of Ruby's module behaviour, and is probably worth documenting in CONTRIBUTING.md to rationalize the unusual structure of lib/shrine.rb.

In considering and trying to get started implementing derivatives, potential race conditions are seeming one of the hardest parts to me at the moment.  It might be very hard to do this in an ORM-agnostic way.

Have you seen my comment about using SELECT ... FOR UPDATE locking? That could be an elegant solution. This is how I imagine it (in Sequel):

# generate derivates

photo.db.transaction do

  original_image = photo.image

  photo.lock! # refreshes the record using a SELECT ... FOR UPDATE query

  raise Sequel::Rollback if photo.image != original_image # attachment has changed

  photo.image_attacher.add_derivates(hash)

  photo.save

end

The locking mechanism would be pushed down to the #add_derivates method, this is just to show how it would work. Of course, Mongoid and other non-SQL ORMs would still be prone to race conditions, with them we could still refresh to minimize chance of race conditions (but there would be no locking); this is the current behaviour for all ORMs when background plugin is loaded (see Attacher#swap).

If derivatives are not in the same column as the original, they no longer need to be a part of the `process` step, or even really a hard-coded part of the uploader at all. You could theoretically hook into any point to generate derivatives, and then just call `add_derivative(:something, io)` or something  – or launch your own bg job in after_commit (say if you wanted promotion in foreground but derivs in bg) just fetched the record and then called `add_derivative`.

Yep, that was also my thinking behind the derivates API I proposed.

Incidentally, I believe there is in fact a race condition in the current backgrounding plugin, and no way to get around a race condition without using either optimistic or pessimistic locking. I may file a ticket on that.

Yes, you're right, we should fix that anyway regardless of the derivates plugin, good catch. I have been aware of that, and I recall I have tried solving that with optimistic locking, but gave up on the idea (probably because of the additional column users would need to add). Probably should have documented the possible race condition then.

But I haven't tried pessimistic locking, probably because I didn't understand it until I saw it explained in Sequel documentation recently, and now it seems like a good solution for eliminating the race condition.

I’m currently trying setting a goal of derivatives much like your suggest in my GH Issue – two columns, with actual two attachers/attachments, with the one containing the actual derivatives actually still using something much like the `versions` plugin to make it into a hash of things.

I would really prefer to avoid the path of two attachers/attachments, as I explained in my last comment in the original thread. My comment in #254 was just a suggestion on how you can currently achieve a similar behaviour, until the improved derivates plugin is written.

Also, the versions plugin did one fatal mistake, which is to override Shrine#upload and allow it to accept a list of versions, in addition to a single file. That adds complexity, and is the main reason why Shrine#_store and Shrine#_delete methods exist. We should keep Shrine#upload accepting only a single file, and move the responsibility of handling of versions for upload/delete up to Shrine::Attacher.

[Janko Marohnić]

Hiren, if you're curious, here is a complete code example showing how the race condition can currently happen, and I've also added how it would be fixed with a SELECT ... FOR UPDATE lock:

https://gist.github.com/janko-m/da7bf374872dad28fcda0713f7c98ee7

I've omitted any versions/derivates, as this can happen with regular promotion without any processing. Basically, what can happen is that promotion of an old attachment in the background can overwrite a new attachment that was assigned. Shrine's backgrounding plugin already checks whether the attachment has changed after processing/storing (but before saving the data to the column), to minimize the chance for a race condition to happen, but it's still theoretically possible. This race condition can only be caused if the attachment of the same record is updated in a short time period.

Jonathan, is this what you meant?

[Hiren Mistry]

Janko,

Yes I'm curious and was going to ask if it was that case. Thank you! :)

Hiren

[Rochkind, Jonathan]

Aha, I hadn’t noticed  the design.md doc before, thanks, that helps I’ll spend more time with it!

 

> Yeah, that's the use case why the processing plugin and versions are separate plugins. But I would now prefer if people would trigger this processing themselves when they want to, e.g:

 

Hmm, I don’t completely follow this. But I like being able to have a single-image processing step be ‘automatic’.  Not totally sure how to make this so with your suggested code. Maybe there’s a way to do it using ‘hooks’ though? 

 

If you got rid of the processing plugin, would you get rid of/deprecate the entire “process”/”processed” method?

> I would like to do that, but then it wouldn't be possible for plugins to override base functionality. If core methods were defined directly on the classes, then modules included into those classes wouldn't be able to override those methods. Putting the core functionality into modules and including them into core classes as 1st modules allows subsequent modules to override methods from the base modules.

 

Aha, I see what you mean. You can potentially get around this with the newer `prepend` functionality in ruby, but I’m not sure if that’s wise or not. I see your point and the justification for core being a plugin, okeydoke for now!

 

> Have you seen my comment

 about using SELECT ... FOR UPDATE locking? That could be an elegant solution. This is how I imagine it (in Sequel):

I had not! But yes, something along those lines is what we need. I’m not sure if that kind of pessimistic locking is better than a kind of optimistic locking, but either way some kind of ORM-specific behavior is needed. Would this be built in to a ORM plugins or something the local implementer did themselves?

 

If we work it out first for the backgrounding plugin, without versions (where I think it’s also needed, and I think there’s currently a race condition), it might be a good first step to figuring out how to use it for somewhat more complicated versions/derivatives use case.

 

> Yes, you're right, we should fix that anyway regardless of the derivates plugin, good catch. I have been aware of that, and I recall I have tried solving that with optimistic locking, but gave up on the idea (probably because of the additional column users would need to add). Probably should have documented the possible race condition then.

 

Technically it may not necessarily need new columns, you could optimistic lock _on_ the shrine _data column… maybe.  Not sure if this works. Works more easily if you have jsonb in postgres. Is a pain in ActiveRecord API regardless.

 

> Yep, that was also my thinking behind the derivates API I proposed.

 

Doh, I’m sorry, I think I’m losing track of things. Can you point me to where you proposed a derivatives API, and I’ll make sure to save it and not lose it again?

 

 

> I would really prefer to avoid the path of two attachers/attachments, as I explained in my last comment in the original thread. My comment in #254 was just a suggestion on how you can currently achieve a similar behaviour, until the improved derivates plugin is written.

 

> Also, the versions plugin did one fatal mistake, which is to override Shrine#upload and allow it to accept a list of versions, in addition to a single file. That adds complexity, and is the main reason why Shrine#_store and Shrine#_delete methods exist. We should keep Shrine#upload accepting only a single file, and move the responsibility of handling of versions for upload/delete up to Shrine::Attacher.

Hmm, I’m honestly just kinda lost in how to implement this. There’s a LOT going on here, I’m getting kind of conceptually overwhelmed.

 

I might start out with the two uploaders just cause it makes it a lot less overwhelming for me, and allows me to re-use a lot more of the existing code – since all existing shrine code assumes one column.

 

Okay, this is all super helpful though, thank you!

 

I’m going to start by, for myself, really going through everything that’s happening _now_ in the code, and charting it out and maybe writing it up in a blog post in case it’s useful to anyone else.

 

Jonathan

[Janko Marohnić]

Hmm, I don’t completely follow this. But I like being able to have a single-image processing step be ‘automatic’.  Not totally sure how to make this so with your suggested code. Maybe there’s a way to do it using ‘hooks’ though?

If you got rid of the processing plugin, would you get rid of/deprecate the entire “process”/”processed” method? 

From people's questions in the google group I felt that the process plugin was often a source of confusion. "When is it triggered exactly? Can I process the same attachment multiple times? How can I pass variables from my controller to the process block?"

And I don't like the fact that it's currently part of the uploader, as the file can just be processed before Shrine#upload. And there is nothing to do, the Shrine#process method is literally empty, which was a code smell to me. Two main reason why I implemented it like that is (a) to make it automatic and (b) to automatically log processing durations; but neither of this reasons are strong enough IMO. And also, because Shrine#upload is called multiple times, we need to pass in the :action parameter to distinguish when processing should actually be called.

That's why I'm not very fond of the processing plugin. It's probably not necessary to remove it, but I would like to make explicit processing easy, so that users can just use that as an alternative.

Aha, I see what you mean. You can potentially get around this with the newer `prepend` functionality in ruby, but I’m not sure if that’s wise or not. I see your point and the justification for core being a plugin, okeydoke for now!

Since I was copying the plugin system for Roda, which still supports Ruby 1.9, I haven't actually seriously considered Module#prepend (I remember thinking about it, but for some reason I discarded it). That's a very intriguing idea, it would be so great for readability if we could define base functionality in classes directly, then we'd be able to naturally split `lib/shrine.rb` into multiple files.

At the moment I'm all for switching to the Module#prepend implementation. Only I would wait until Shrine 3.0 for this, just because it could cause backwards incompatibility for anyone that's currently overriding methods by using `Shrine::Attachment.include MyMonkeyPatches`.

I’m not sure if that kind of pessimistic locking is better than a kind of optimistic locking, but either way some kind of ORM-specific behavior is needed. Would this be built in to a ORM plugins or something the local implementer did themselves?

Locking would be implemented in ORM plugins. For ORMs that don't support such locking we could keep the fallback to the current refresh-and-update approach that we have in the backgrounding plugin.

Doh, I’m sorry, I think I’m losing track of things. Can you point me to where you proposed a derivatives API, and I’ll make sure to save it and not lose it again?

Np, it was at the bottom of this comment :)

Kind regards,

Janko

[Rochkind, Jonathan]

> And I don't like the fact that it's currently part of the uploader, as the file can just be processed before Shrine#upload. And there is nothing to do, the Shrine#process method is literally empty, which was a code smell to me. Two main reason why I implemented it like that is (a) to make it automatic and (b) to automatically log processing durations; but neither of this reasons are strong enough IMO. And also, because Shrine#upload is called multiple times, we need to pass in the :action parameter to distinguish when processing should actually be called.

 

Hmm, I like that the current processing plugin lets me do it as part of promotion, even when I’m using backgrounding to make promotion in the bg. And I like that it’s part of the uploader, to keep all my code in one place. 😊  But curious to see what else you come up with.

 

> Since I was copying the plugin system for Roda, which still supports Ruby 1.9, I haven't actually seriously considered Module#prepend (I remember thinking about it, but for some reason I discarded it). That's a very intriguing idea, it would be so great for readability if we could define base functionality in classes directly, then we'd be able to naturally split `lib/shrine.rb` into multiple files.

 

I realize there’s a potentially even more straightforward option than Module#prepend.

 

You’ve just got to make your “core” implementations superclasses. 

 

Eg, `Attachment < Module` at https://github.com/shrinerb/shrine/blob/master/lib/shrine.rb#L42

 

Becomes instead simply `Attachment < Shrine::AttachmentCore` or whatever. The “core” is in a superclass instead of in the first plugin set of modules include’d.  Same for all other objects.   Cause we don’t have ActiveSupport::Concern, there might be separate bases for the class methods, I dunno, would have to be played with. But the superclass approach should be able to accomplish everything the prepend approach does, I think.

 

No confusing prepend, everything in core is still over-rideable by plugins, no backwards incompatibility I can think of either.

 

I do think it would make the code easier to read. On the other hand, I’ve gotten used to having all shrine core code in ONE file (shrine.rb) I can search. I suppose you could still do that with a superclass approach, although it’d be unconventional.  Probably not worth it, even though I’ve seen the benefits of the idiosyncracy, to have the idiosyncracy. 😊

 

Jonathan