862912 - Rework Thimble Save/Publish according to Webmaker2 workflow, use S3

Status: RESOLVED FIXED

(Webmaker Graveyard :: Thimble, defect)

Description

[Pomax]

Right now we're storing a scheme with data columns:

rawData : the raw HTML a user published
sanitizedData : the cleaned up, safe-i-fied data
finalizedData : the cleaned up HTML, with metrics/GA added in

We can probably split this up instead:

rawData/sanitizedData => DB
finalizedDAta => AWS

Comment 1

[Pomax]

I know popcorn has an s3 writer it uses, is that a node_module we can use for thimble, too?

Comment 2

[David Humphrey (:humph)]

It's not, but can be made so.  I wrote it, so I'm probably able to do this fastest.

The only other approach I can see here, and I would like to consider, is not having this code live in our tools at all, and instead making a node app to do publishing as a separate thing.  So publish.webmaker.org would have a simple API for creating/removing content from S3, and our tools would just use it.

I don't think Jon's a fan of this approach.

Comment 3

[Pomax]

I'd be fine with that, since we're already doing that for Bleach, too. As long as we have a nice node module wrapping it, and can be run localhost if necessary I guess (so that no wifi doesn't break the process during testing)

Comment 4

[Jon Buckley]

So, I don't think we use the LocalFS/S3FS code from Popcorn because we should only support writing to S3. My reasoning here is that your local filesystem and S3 are different enough from each other that you can't abstract away the differences between them, and we're going to cause bugs in the S3FS layer because we'll all use LocalFS on dev. If we force devs to use S3, we'll catch bugs before they land on master. AWS has a free tier, and all webmaker staff will get AWS accounts so I think this is a workable approach.

As for the idea of having a publishing app, I don't see the point for having a REST API to S3 when S3 itself already has a REST API that we interact with. If there's other reasons for wanting to do this, I'd be happy to discuss this idea.

Comment 5

[David Humphrey (:humph)]

I'm going to morph this ticket and we can just work on the changes to Thimble here vs. me filing a new bug.

Comment 6

[David Humphrey (:humph)]

(In reply to Jon Buckley [:jbuck] from comment #4)
> So, I don't think we use the LocalFS/S3FS code from Popcorn because we
> should only support writing to S3. My reasoning here is that your local
> filesystem and S3 are different enough from each other that you can't
> abstract away the differences between them, and we're going to cause bugs in
> the S3FS layer because we'll all use LocalFS on dev. If we force devs to use
> S3, we'll catch bugs before they land on master. AWS has a free tier, and
> all webmaker staff will get AWS accounts so I think this is a workable
> approach.

Sounds sane.  The only caution I have here is that it means that it's harder for non-staff to do this.  Do we need a bug filed on getting everyone accounts for S3, and a dev bucket(s) setup? 

> As for the idea of having a publishing app, I don't see the point for having
> a REST API to S3 when S3 itself already has a REST API that we interact
> with. If there's other reasons for wanting to do this, I'd be happy to
> discuss this idea.

If we don't do filesystem + S3, then I agree, let's just go direct to S3 from each app.

Comment 7

[Jon Buckley]

(In reply to David Humphrey (:humph) from comment #6)
> Sounds sane.  The only caution I have here is that it means that it's harder
> for non-staff to do this.  Do we need a bug filed on getting everyone
> accounts for S3, and a dev bucket(s) setup? 

AWS has a free tier available for one year, and I'm happy to make accounts for anyone that asks me. I'll co-ordinate with JP on the best way to create accounts for this purpose.

Comment 8

[Pomax]

Attachment: https://github.com/mozilla/thimble.webmaker.org/pull/24

publishing through S3, implemented using the fakes3 program for offline testing

Comment 9

[David Humphrey (:humph)]

Comment on attachment 740966 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/24

A few nits, suggestions in the pull request.  With those fixed, this is fine.  If you want Jon to also have a look, feel free to do another review.

Comment 10

[Pomax]

I'll fix them and then ask jon for a review. Adding the bug numbers is a good idea.

Comment 11

[Jon Buckley]

Comment on attachment 740966 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/24

r+ with some nits noted in the pull request.

Comment 12

[Pomax]

Asking for re-review, as the S3 code got rebased onto the MakeAPI code done by Scott. I don't see anything breaking, grunt doesn't complain (although we need to make grunt stricter), and all the patches landed while working on this have been worked in.

Comment 13

[Jon Buckley]

Comment on attachment 740966 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/24

r+ with nits noted in the pull request

Comment 14

[Pomax]

landed.