Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add io/fs#FS Driver #471 #472

Merged
merged 15 commits into from
Nov 18, 2020
Merged

Add io/fs#FS Driver #471 #472

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
489e6d3
Add io/fs#FS Driver #471
johejo Nov 12, 2020
65cbb0f
Refactor iofs.Iofs to be independent of httpfs
johejo Nov 13, 2020
e18e378
Rename Iofs to IoFS, accepts fs.FS and use fs.DirEntry.Info() to undo…
johejo Nov 14, 2020
e9ae08d
fix documents and migration directory for testing
johejo Nov 15, 2020
416f878
Fix iofs.Driver so that it can be used by embedding it like httpfs.Pa…
johejo Nov 15, 2020
45ec3dc
iofs.Driver closes the file system if possible
johejo Nov 15, 2020
3af8acf
Rename receiver
johejo Nov 15, 2020
0e9937b
Refactor file driver to use iofs.Driver
johejo Nov 15, 2020
7fb518d
iofs build constraint is not needed so remove it
johejo Nov 15, 2020
3c0290b
Properly return an error in a corner case
johejo Nov 15, 2020
2cd12a6
Increase golangci-lint timeout
johejo Nov 15, 2020
ca6628d
Fix example code
johejo Nov 15, 2020
58e9837
Revert "Increase golangci-lint timeout"
johejo Nov 17, 2020
321662e
unexport driver, add iofs.PartialDriver, remove type alias from file …
johejo Nov 17, 2020
02da3b6
do not panic in Open, return error
johejo Nov 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
run:
# timeout for analysis, e.g. 30s, 5m, default is 1m
timeout: 5m
timeout: 2m
linters:
enable:
#- golint
Expand Down
2 changes: 0 additions & 2 deletions source/file/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ func init() {
source.Register("file", &File{})
}

type File = file

func parseURL(url string) (string, error) {
u, err := nurl.Parse(url)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions source/file/file_go115.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import (
"github.com/golang-migrate/migrate/v4/source/httpfs"
)

type file struct {
type File struct {
httpfs.PartialDriver
url string
path string
}

func (f *file) Open(url string) (source.Driver, error) {
func (f *File) Open(url string) (source.Driver, error) {
p, err := parseURL(url)
if err != nil {
return nil, err
Expand Down
8 changes: 4 additions & 4 deletions source/file/file_go116.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@ import (
"github.com/golang-migrate/migrate/v4/source/iofs"
)

type file struct {
iofs.Driver
type File struct {
iofs.PartialDriver
url string
path string
}

func (f *file) Open(url string) (source.Driver, error) {
func (f *File) Open(url string) (source.Driver, error) {
p, err := parseURL(url)
if err != nil {
return nil, err
}
nf := &file{
nf := &File{
url: url,
path: p,
}
Expand Down
2 changes: 1 addition & 1 deletion source/iofs/example_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
var fs embed.FS

func Example() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for using executable examples!

d, err := iofs.NewDriver(fs, "testdata/migrations")
d, err := iofs.New(fs, "testdata/migrations")
if err != nil {
log.Fatal(err)
}
Expand Down
47 changes: 28 additions & 19 deletions source/iofs/iofs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,13 @@ import (
"github.com/golang-migrate/migrate/v4/source"
)

// Driver is a source driver that wraps io/fs#FS.
type Driver struct {
migrations *source.Migrations
fsys fs.FS
path string
type driver struct {
PartialDriver
}

// NewDriver returns a new Driver from io/fs#FS and a relative path.
func NewDriver(fsys fs.FS, path string) (source.Driver, error) {
var i Driver
// New returns a new Driver from io/fs#FS and a relative path.
func New(fsys fs.FS, path string) (source.Driver, error) {
var i driver
if err := i.Init(fsys, path); err != nil {
return nil, fmt.Errorf("failed to init driver with path %s: %w", path, err)
}
Expand All @@ -31,13 +28,25 @@ func NewDriver(fsys fs.FS, path string) (source.Driver, error) {

// Open is part of source.Driver interface implementation.
// Open panics when called directly.
func (d *Driver) Open(url string) (source.Driver, error) {
panic("iofs: Driver does not support open with url")
func (d *driver) Open(url string) (source.Driver, error) {
panic("iofs: driver does not support open with url")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't panic and instead return an error: return nil, errors.New("Open() cannot be called on the iofs passthrough driver")

}

// PartialDriver is a helper service for creating new source drivers working with
// io/fs.FS instances. It implements all source.Driver interface methods
// except for Open(). New driver could embed this struct and add missing Open()
// method.
//
// To prepare PartialDriver for use Init() function.
type PartialDriver struct {
migrations *source.Migrations
fsys fs.FS
path string
}

// Init prepares not initialized IoFS instance to read migrations from a
// io/fs#FS instance and a relative path.
func (d *Driver) Init(fsys fs.FS, path string) error {
func (d *PartialDriver) Init(fsys fs.FS, path string) error {
entries, err := fs.ReadDir(fsys, path)
if err != nil {
return err
Expand All @@ -50,7 +59,7 @@ func (d *Driver) Init(fsys fs.FS, path string) error {
}
m, err := source.DefaultParse(e.Name())
if err != nil {
return err
continue
}
file, err := e.Info()
if err != nil {
Expand All @@ -72,7 +81,7 @@ func (d *Driver) Init(fsys fs.FS, path string) error {

// Close is part of source.Driver interface implementation.
// Closes the file system if possible.
func (d *Driver) Close() error {
func (d *PartialDriver) Close() error {
c, ok := d.fsys.(io.Closer)
if !ok {
return nil
Expand All @@ -81,7 +90,7 @@ func (d *Driver) Close() error {
}

// First is part of source.Driver interface implementation.
func (d *Driver) First() (version uint, err error) {
func (d *PartialDriver) First() (version uint, err error) {
if version, ok := d.migrations.First(); ok {
return version, nil
}
Expand All @@ -93,7 +102,7 @@ func (d *Driver) First() (version uint, err error) {
}

// Prev is part of source.Driver interface implementation.
func (d *Driver) Prev(version uint) (prevVersion uint, err error) {
func (d *PartialDriver) Prev(version uint) (prevVersion uint, err error) {
if version, ok := d.migrations.Prev(version); ok {
return version, nil
}
Expand All @@ -105,7 +114,7 @@ func (d *Driver) Prev(version uint) (prevVersion uint, err error) {
}

// Next is part of source.Driver interface implementation.
func (d *Driver) Next(version uint) (nextVersion uint, err error) {
func (d *PartialDriver) Next(version uint) (nextVersion uint, err error) {
if version, ok := d.migrations.Next(version); ok {
return version, nil
}
Expand All @@ -117,7 +126,7 @@ func (d *Driver) Next(version uint) (nextVersion uint, err error) {
}

// ReadUp is part of source.Driver interface implementation.
func (d *Driver) ReadUp(version uint) (r io.ReadCloser, identifier string, err error) {
func (d *PartialDriver) ReadUp(version uint) (r io.ReadCloser, identifier string, err error) {
if m, ok := d.migrations.Up(version); ok {
body, err := d.open(path.Join(d.path, m.Raw))
if err != nil {
Expand All @@ -133,7 +142,7 @@ func (d *Driver) ReadUp(version uint) (r io.ReadCloser, identifier string, err e
}

// ReadDown is part of source.Driver interface implementation.
func (d *Driver) ReadDown(version uint) (r io.ReadCloser, identifier string, err error) {
func (d *PartialDriver) ReadDown(version uint) (r io.ReadCloser, identifier string, err error) {
if m, ok := d.migrations.Down(version); ok {
body, err := d.open(path.Join(d.path, m.Raw))
if err != nil {
Expand All @@ -148,7 +157,7 @@ func (d *Driver) ReadDown(version uint) (r io.ReadCloser, identifier string, err
}
}

func (d *Driver) open(path string) (fs.File, error) {
func (d *PartialDriver) open(path string) (fs.File, error) {
f, err := d.fsys.Open(path)
if err == nil {
return f, nil
Expand Down
2 changes: 1 addition & 1 deletion source/iofs/iofs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
func Test(t *testing.T) {
//go:embed testdata/migrations/*.sql
var fs embed.FS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just curious--how did this even work?

Per the golang docs:

The //go:embed directive can be used with both exported and unexported variables, depending on whether the package wants to make the data available to other packages. It can only be used with global variables at package scope, not with local variables.

To me this looks like a local variable, not a global variable at package scope... What am I missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local variables were still supported when implementing this PR.
golang/go#43216
It is a remnant of that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thank you!

d, err := iofs.NewDriver(fs, "testdata/migrations")
d, err := iofs.New(fs, "testdata/migrations")
if err != nil {
t.Fatal(err)
}
Expand Down