activewarehouse / activewarehouse-etl

Extract-Transform-Load library from ActiveWarehouse

MIT License

240 stars 102 forks source link

add last completed id option #104

Closed sgrgic closed 12 years ago

sgrgic commented 12 years ago

Hi Thibaut,

Review this since we are using it in our application for some time. If you find it useful as we are please merge it to your project.

Thanks, Sinisa.

sgrgic commented 12 years ago

Note: I'm merging now our changes to your latest version so this is not tested code. Maybe you can merge what you think is ok once we verify everything is up and running.

josephbridgwaterrowe commented 12 years ago

Oh, I would also like to see these changes merged!

thbar commented 12 years ago

A couple of notes on why I can't merge it as is:

the "\N" is MySQL only I think and would need to be passed as a parameter to let the user disable or enable the behaviour (and this would require tests as well!)
the completed_id, as discussed with @sgrgic, is one possible scenario of something with more scenarios (ie: people extracting based on timestamp, created_at, modified_at etc); this would need more work on a separate branch before merging.

Quick thoughts!

sgrgic commented 12 years ago

I think we can close this pull request. Any reasons not to?

Sinisa.

thbar commented 12 years ago

As is, I think you are right; I'd prefer that we work on smaller pull-requests if it's possible for you.

Closing for now!

josephbridgwaterrowe commented 12 years ago

@sgrgic,

Thanks for information regarding mysql.

Are you planning to create new forks/pull requests for the various changes you have made, or @thbar will you be introducing some of these changes directly?

Thanks guys!

sgrgic commented 12 years ago

Not sure, as Thibaut says we need to split those changes to smaller pull request. Also a lot of changes are use case specific so we can't merge them to master.

Regards, Sinisa.

josephbridgwaterrowe commented 12 years ago

Thanks, well I would be happy to help with creating the forks and pull requests to get the common features back into the master...

Which changes would you say were use case specific?

thbar commented 12 years ago

@josephbridgwaterrowe I've started extracting the specs for those individual bits.

First one: #107 on nil for FileDestination.

I'm going through the commits provided here to create new issues based on that so we can move on!

thbar commented 12 years ago

108: allow to pass extra arguments to MySQL streamer

thbar commented 12 years ago

109: add support for :accept_nil on ForeignKeyLookupTransform

josephbridgwaterrowe commented 12 years ago

Hi @thbar sorry I've been out of the loop, things have been hectic for me... were you addressing myself or @sgrgic?