Getters refactoring #255
Getters refactoring #255
Conversation
| // When the ssh: protocol is used explicitly, we recognize it as | ||
| // URL form rather than SCP-like form, so the part after the colon | ||
| // is a port number, not part of the path. | ||
| "git::ssh://git@git.example.com:2222/hashicorp/foo.git", |
There was a problem hiding this comment.
This test was moved to get_git_test.go
| // is nil, then the default Getters variable will be used. | ||
| Getters map[string]Getter | ||
| Getters []Getter |
|
|
||
| // Make sure that we don't start with "/" since we add that below. | ||
| if path[0] == '/' { | ||
| path = path[1:] | ||
| } | ||
| return fmt.Sprintf("file:///%s", path) |
There was a problem hiding this comment.
Was this causing a bug ? What makes this change possible ?
There was a problem hiding this comment.
This was only for telling the client that the FileDetector detected the source string as valid. In the previous code, once this is back to the client, the client was going to remove the forced "file" to use as a key to look for the getter on the map. No longer necessary.
There was a problem hiding this comment.
Oh right ! 💯 thanks for explaining !
| }, | ||
| { | ||
| "www.googleapis.com/storage/v1/foo/bar.baz", | ||
| "gcs::https://www.googleapis.com/storage/v1/foo/bar.baz", | ||
| "https://www.googleapis.com/storage/v1/foo/bar.baz", |
There was a problem hiding this comment.
Yes. This is the same situation as the FileDetector you asked above. This was for the client.go to know which getter to use.
|
Nice, this is such a big PR 🙂 ! I tried looking at everything with a bit more attention to the smb code and nice ! |
6bcaa35 to
0872741
Compare
client.go
Outdated
| @@ -39,6 +35,17 @@ type GetResult struct { | |||
| Dst string | |||
| } | |||
|
|
|||
| type getError struct { | |||
| // when fatal is true it means that something when wrong with | |||
There was a problem hiding this comment.
It would be good to have this comment added to any exported method that returns a getError so that it is clear what to expect from the error. Or maybe just export this type as in GetError and add the comment above the type so that it is available via godoc. What do you think?
There was a problem hiding this comment.
Right now this one is only returned by the get method which is not exported. Do you think exporting getError is necessary anyways?
I'm updating the comment to make it clear that is used by get.
There was a problem hiding this comment.
Oh in that case you can disregard my comment. I don't think exporting it is necessary if its only used by unexported methods.
0872741 to
1ab379b
Compare
This is an extension of #240.
Client refactoring
The PR refactors the client code to make it try all valid Getters for a given source string. Each Getter will try to detect a valid source string and if the detection is successful it will then try to download the artifact. The client loop over the Getters list until the download is successful.
This doesn't change the behavior of the existing Getters and improves the code. Multiple Getters can be created as good candidates to download the same source string but using different possibilities, whenever this is necessary, such as the SmbClientGetter and the SmbMountGetter, also implemented in this PR. If there's a new and different approach to retrieve an artifact and you still want to fall back to the current Getter. This allows us to add it as a new Getter without affecting the existing one.
SMB Getters
This PRs adds a SmbClientGetter and a SmbMountGetter to download files from shared folders using SMB protocol.
This PR also adds the necessary config for running the tests. This includes:
This is motivated by issues hashicorp/packer#8729 and hashicorp/packer#8552 caused by
hashicorp/packer#7627