Closed alanking closed 3 weeks ago
Ah yes. I'll run black just before we # this because it's going to make rebasing very tricky later should I need to squash things.
Nevermind, I found a way to thread the needle. Had to run the formatter on the test suite before new changes came in and things work out pretty cleanly. Force-pushed because still in draft. Will test S3 shortly...
nice.
Had to make a tweak and then was able to successfully sync an S3 bucket with Minio in the demo Docker Compose project. So, I guess this is ready for review.
Caught an issue in the latest changes... Tests are passing again.
Tests are passing with latest changes
Rebased and squashed. Didn't mean to push, though. Sorry about that, reviewers...
very shady.
We got an approval from @FifthPotato.
Do you feel this is ready for squashing/merging? Is there anything else left to do for this PR?
I think this is ready. The goal was to keep the behavior unchanged as much as possible and the tests are passing, so I think adding anything else in this PR is unnecessary if we're all good with the existing changes.
Sounds good to me.
Please squash to taste.
Squashed
Addresses #211 Addresses #262 Addresses #269 Addresses #272 Addresses #274
Some notes on this PR...
I don't think we need any new tests because the goal is for the new implementation to be identical in behavior to the old implementation besides having new Celery application and task names.
Also, I realize this is a big change, so here's a few areas where eyeballs are most needed (IMO):
@app.task
)Will leave in draft til I can test with S3.