Add support for loading URLs.#356
Conversation
Codecov Report
@@ Coverage Diff @@
## master #356 +/- ##
==========================================
+ Coverage 58.63% 58.66% +0.03%
==========================================
Files 56 56
Lines 2833 2845 +12
==========================================
+ Hits 1661 1669 +8
- Misses 944 946 +2
- Partials 228 230 +2
Continue to review full report at Codecov.
|
chris-crone
left a comment
chris-crone
left a comment
There was a problem hiding this comment.
LGTM, just missing a test
| if err != nil { | ||
| return nil, errors.Wrap(err, "failed to download "+url) | ||
| } | ||
| return LoadFromSingleFile(url, resp.Body, ops...) |
There was a problem hiding this comment.
if resp.Body != nil{
defer resp.Body.Close()
}| if err != nil { | ||
| return nil, errors.Wrap(err, "failed to download "+url) | ||
| } | ||
| return LoadFromSingleFile(url, resp.Body, ops...) |
There was a problem hiding this comment.
What if the response is anything but 200 ? We should return an error here before calling LoadFromSingleFile.
There was a problem hiding this comment.
I assumed it would err but you're right it doesn't. Fixed.
mnottale
force-pushed
the
load-from-url
branch
from
2f7ac40 to
192ed4c
Compare
vdemeester
left a comment
vdemeester
left a comment
There was a problem hiding this comment.
How do we handle docker-app render https://foo.bar/apps/my.dockerapp.tar.gz (or even a my.dockerapp that is a tarball) ?
We shouldn't make the assumption that the downloaded URL is always a single-file app
|
We don't. This is out of scope for this PR. |
silvin-lubecki
left a comment
silvin-lubecki
left a comment
There was a problem hiding this comment.
LGTM, with a test? 😅
mnottale
force-pushed
the
load-from-url
branch
from
192ed4c to
5a36d2c
Compare
|
e2e test added |
|
@vdemeester are you OK with handling other package formats in a followup? |
Signed-off-by: Matthieu Nottale <matthieu.nottale@docker.com>
mnottale
force-pushed
the
load-from-url
branch
from
5a36d2c to
1dae8e1
Compare
5 participants
Signed-off-by: Matthieu Nottale matthieu.nottale@docker.com
- What I did
Add support from loading docker-app from an URL (http or https).
- How I did it
Hook Extract(), add a loader.LoadFromURL.
- How to verify it
docker-app render someURL- Description for the changelog