search

mattes / migrate

Database migrations. CLI and Golang library.
Other
2.29k stars 326 forks source link

source/google-cloud-storage: implement the driver #227

Closed fsouza closed 7 years ago

fsouza commented 7 years ago

@mattes please let me know what you think.

I wanted to have a better test coverage on this, but testing the Open method is hard because of the call to storage.NewClient.

Closes #163.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 53.738% when pulling da89656fdc7425aeeb5ac3be2801cfc752159ba4 on fsouza:gcs-source into 448a36c5cbdcc34f348f605a5080b23629c7fbbc on mattes:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 53.738% when pulling da89656fdc7425aeeb5ac3be2801cfc752159ba4 on fsouza:gcs-source into 448a36c5cbdcc34f348f605a5080b23629c7fbbc on mattes:master.

fsouza commented 7 years ago

I can also work on the s3 driver, the code would be very similar, as the concepts are the same. Just let me know if you like the structure of this PR.

mattes commented 7 years ago

This looks great. I have a question around https://github.com/mattes/migrate/pull/227/files#diff-4178ab4a0dc6cc9bd3e00c7d70f1d1bfR48. Does the GCS API offer a way to return the next file without loading them all upfront?

fsouza commented 7 years ago

@mattes we could use prefixes for both GCS and S3 on this, but I don't think it is safe to assume the prefix of the migration (e.g. 1 vs 01 vs 001 vs 0001), is it?

Aside from that, both systems support pagination, and the S3 sdk allows me to set page size to 1 (see #228), but I wasn't able to find a similar option in the GCS sdk (the API supports that, but the sdk doesn't expose the option).

Since the GCS client uses iterators for the file list (and underneath it paginates requests, but I think I don't have control on the size of the page), we could save the iterator internally and consume it as the user calls Next (and save the list to support Prev). This way loadMigrations would make the call and save the iterator as an attribute in the struct.

mattes commented 7 years ago

See my comment for the AWS driver. Let's merge it the way it is for now and then in the future we can iterate and improve.