history: add history import command #3039
Conversation
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
tonistiigi
force-pushed
the
history-import
branch
from
eaa9bd5 to
fea7459
Compare
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
| cmd := &cobra.Command{ | ||
| Use: "import [OPTIONS] < bundle.dockerbuild", | ||
| Short: "Import a build into Docker Desktop", | ||
| Args: cobra.NoArgs, |
There was a problem hiding this comment.
I think we should use args as file input instead of a flag and require at least one arg. Multiple files should be supported like Docker Desktop does atm imo:
docker buildx history import FILE...
There was a problem hiding this comment.
That's not usually the convention I think when the file is optional and defaults to stdin. If file is arg then for stdin, the user should write import -
There was a problem hiding this comment.
Importing multiple files together, I'm ok with.
There was a problem hiding this comment.
Ah indeed I missed it was reading from stdin. Sounds good to support multiple --file then
commands/history/import.go
Outdated
| if i == 0 { | ||
| err = browser.OpenURL(url) | ||
| } |
There was a problem hiding this comment.
If multiple files are provided I think we should skip this.
There was a problem hiding this comment.
Because who knows what user wants to see? Is it the last one or the first one? But I guess that's fine to open the very first imported record in Docker Desktop.
There was a problem hiding this comment.
Although I wonder if there are more than one record imported we could instead open the Builds view and filter by imported builds 🤔
tonistiigi
force-pushed
the
history-import
branch
from
bc7bb88 to
6f08838
Compare
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
tonistiigi
force-pushed
the
history-import
branch
from
6f08838 to
cadf4a5
Compare
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
| if os.Getenv("WSL_DISTRO_NAME") != "" { | ||
| return "unix://" + filepath.Join(wslSocketPath, socketName), nil | ||
| } |
There was a problem hiding this comment.
Pushed this change for testing WSL support but doesn't work. I'm taking a look if this can be solved but in the meantime we could disable support for import command on WSL and explain that it should be invoked on Windows host instead while this is being investigated.
part of #2711