search

bluelabsio / s3-stream

Akka Streaming Client for S3 and Supporting Libraries
Other
16 stars 6 forks source link

Figure out long-term process for this project #14

Closed joearasin closed 7 years ago

joearasin commented 8 years ago

@jypma @jasonmartens

I'd like work out a long-term plan for maintaining this project. As we have seen, "me as commit gatekeeper" is a rather unsustainable plan for getting changes merged in and it would make sense to figure out what we want the scope of this project to be, a feature-roadmap, and what we want the processes to be for architectural decisions, merging in PRs, etc.

jasonmartens commented 8 years ago

I'm primarily interested in using it as a streams-friendly AWS SDK, for some small subset of things I frequently need to do on AWS. I would like to add the ability to copy things between S3 buckets, but after that it's pretty much feature complete for me.

I don't feel too strongly about architecture decisions. I don't see this growing very large, so I'm not sure it's going to be a problem. but if others feel strongly about how things should be done, I'm open to listening. That said, I would rather see working code committed than blocked because it doesn't fit "the architecture".

jypma commented 8 years ago

I'll ask some of the akka guys that are working in the same direction. Maybe we can put this into https://github.com/akka/akka-stream-contrib, for example.

joearasin commented 8 years ago

@jypma That would be awesome!

@jasonmartens I'm in the same boat, and that's what caused me to write this in the first place. I think "not fitting the architecture" was less of a concern as making decisions as to making a decision as to what features belong / don't belong in this library so as we don't sign ourselves up for responsibility to maintain too many things, how we approach testing, etc.

filosganga commented 8 years ago

I am quite interested in this project, as I have written a very similar one ad we are using in production. will be good to trash our stuff and switch on this one.

joearasin commented 8 years ago

@filosganga That was basically how this came to be. We wanted the functionality, had issues adapting existing stuff to fit our needs, and figured other people had the same problem. As-released now, it's a bit raw, and it will be great to have more eyes on this.

filosganga commented 8 years ago

I am happy to contribute on this even if I don't have much time right now, but I will try

jypma commented 8 years ago

After eye-balling some of the talk over on akka/dev, let's say we go in the direction of akka-stream-contrib. Should we get some of the PR's in first?

jypma commented 7 years ago

@joearasin ping on this. I can make a PR to akka-stream-contrib, but I'd like your OK. Also, since it's in my interest, I'd like to include at least https://github.com/bluelabsio/s3-stream/pull/13 :) What's the best way forward?

filosganga commented 7 years ago

@joearasin @jypma I have solved the signature issue on the Pr #15 will be good to see it merged if you are happy with that.

joearasin commented 7 years ago

Hey everyone -- Sorry about the radio silence. Had to ship something last week and got tied up on that and wasn't able to give this attention.

@jypma No problems with making a PR to akka-stream-contrib. I'd be interested in having someone else's opinion on #13 -- the "maxMaterializations" feels like a bit of a kludge to me, or, at the very least, doesn't really go along with the spirit of Akka streams.

@filosganga Re: #15 -- I'm happy to merge that in as is, or if it's worth adding in the KMS context before merging, we can do that. As far as libraries, I've tended to use spray-json. I asked in akka/dev if there was any party line as to what library should be used.

jypma commented 7 years ago

There's https://github.com/Tradeshift/ts-reaktive/tree/master/ts-reaktive-marshal which I've been writing. It's Java-centric for the moment, but I'm happy to create a Scala variant for the DSL if @filosganga is interested in giving it a shot :-)

filosganga commented 7 years ago

@joearasin @jypma I will try to add the KMS context this week (hopefully today or tomorrow). I would like ideally to avoid pulling in a JSON library.

The encryption context is not even a JSON object but just a flat key-value map, I am not even sure if you can have numbers and/or booleans in there, as in the Java AWS SDK it is a Map<String, String>.

Using a simple string interpolation may be a better choice.

About the #13 I don't love the maxMaterializations either, but also I don't have a better idea at the moment. In my scenario, I have a similar problem as the key is the hash of the content. So I have put in place a mechanism of redirection, that abort the multipart upload if the content has been already uploaded.

jypma commented 7 years ago

It's been a while. @joearasin Are you on board with one of us merging the code into akka-contrib, or possibly https://github.com/akka/alpakka as it gets finalized? I'd rather not step on anyone's toes, plus the target repos have the Apache2 license, which is different from the MIT license of this repo.

joearasin commented 7 years ago

I have zero problems merging it into akka/contrib or alpakka. It looks like this thread is rolling over at alpakka: https://github.com/akka/alpakka/issues/1 -- so that seems like a logical destination.

I need to do a quick check with our lawyers on the license thing -- I can't imagine it being a problem, though.