dizda / CloudBackupBundle

Be able to backup your database(s) and upload it to the cloud (Dropbox, Amazon S3, GoogleDrive, etc.)
MIT License
196 stars 57 forks source link

[W.I.P] Simplify & Refactoring the backup process into a manager #51

Closed dizda closed 9 years ago

dizda commented 9 years ago

Following to the discussion #48

Changes log:

Thanks to @Nyholm.

dizda commented 9 years ago

Guys, what do you think ? @AlmogBaku, @jongotlin, @eymengunay, @vittore, @JoydS, @crash21, @Nyholm, @dim

Nyholm commented 9 years ago

There you go. That was my 50 cent.

Nyholm commented 9 years ago

Should we remove the "Client" and "Processor" suffix? We have DropboxClient but not MySQLDatabase. It is just called "MySQL". Should we rename the classes like:

DropboxClient.php => Dropbox.php CloudAppClient.php => CloudApp.php ZipProcessor.php => Zip.php etc

dizda commented 9 years ago

We should be consistent along our code, so yes we can remove the suffixe.

Nyholm commented 9 years ago

:+1:

ndoulgeridis commented 9 years ago

Is this branch working now or is incomplete? I can help on testing if there is not something else I can do.

Nyholm commented 9 years ago

There are some tests that are broken. If @dizda merge #56 you will see that the Processor tests are broken. There are also missing tests for the managers (and dependency injection).

dizda commented 9 years ago

Do you want me to merge now @Nyholm ?

Nyholm commented 9 years ago

yes please

dizda commented 9 years ago

Here you go.

dizda commented 9 years ago

Ok Processors tests are fixed.

But I don't know what to do with the splitter feature... https://github.com/dizda/CloudBackupBundle/blob/6e04abf01d1d4222f2a1432b16fb43e3b4d5a962/Manager/ProcessorManager.php#L119

dizda commented 9 years ago

Hey guys, can you try everything again please ? We almost finished :)

@crash21 can you try the split feature ? I'm not sure that feature still works

ndoulgeridis commented 9 years ago

Hey, I will try test it but probably not before weekend

Nyholm commented 9 years ago

@crash21, did you manage to do some testing?

ndoulgeridis commented 9 years ago

When I run it I got:

Something went terribly wrong. We could not create a backup

ndoulgeridis commented 9 years ago

Is it possible to create a more detailed message here?

Nyholm commented 9 years ago

You have your detailed messages in your log files.

I added a note to make that more clear. #58.

ndoulgeridis commented 9 years ago

Hmm thats the error:

[2015-03-18 11:57:29] app.CRITICAL: [dizda-backup] Unexpected exception. {"exception":"[object]

(Exception(code: 1): zipsplit warning: Entry is larger than max split size of: 349978\nzipsplit warning: use -n to set split size\nzipsplit error: Entry too big to split, read, or write (folders/app/data/uploads/about/2014-05-11 10.22.27.jpg)\n at ....../vendor/dizda/cloud-backup-bundle/Dizda/CloudBackupBundle/Splitter/ZipSplitSplitter.php:27)"} []

Nyholm commented 9 years ago

Haha, okey. That did not make it any better ;)

Is that the only log? Can you debug the BackupManager?

(I can successfully backup to google drive, btw)

ndoulgeridis commented 9 years ago

Your backup that was successful, was using split feature?

Nyholm commented 9 years ago

Sorry, no. It was not.

ndoulgeridis commented 9 years ago

OK give me some time to debug it

ndoulgeridis commented 9 years ago

Found issue

\Dizda\CloudBackupBundle\Manager\ProcessorManager::split line 167

you have harcoded 350000, this should be split_size variable from config

Nyholm commented 9 years ago

Do you submit an PR or should I?

ndoulgeridis commented 9 years ago

Can you please do it as I don't have something installed here.

Nyholm commented 9 years ago

I've added #60. I put the Splitter as a service. Try now if you get it to work.

Nyholm commented 9 years ago

What is left to do on this PR? Could we create a new branch from master called 1.x and merge this to master and create a beta-release?

Im ready to start running this in production now.

dizda commented 9 years ago

There are only some missing tests for now. The rest is okay.

Good idea @Nyholm, if it's good for everyone I can merge it into master and backup the current version on another branch.

dizda commented 9 years ago

That's it, after one month of refactoring, and hard work from some of you, the bundle is much more better now. Thank you for your help, great job guys!

AlmogBaku commented 9 years ago

Hey guys, Sorry for not responding here (really busy days on the new startup). Anyway- it looks like you did really good job here!

~ Almog


Q: Why is this email five sentences or less? A: http://five.sentenc.es

On Thu, Mar 19, 2015 at 6:10 PM, Jonathan Dizdarevic < notifications@github.com> wrote:

That's it, after one month of refactoring, and hard work from some of you, the bundle is much more better now. Thank you for your help, the bundle is much more better now. Great job guys!

— Reply to this email directly or view it on GitHub https://github.com/dizda/CloudBackupBundle/pull/51#issuecomment-83645858 .