You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by Graeme-Miller <gi...@git.apache.org> on 2017/04/07 10:57:35 UTC
[GitHub] brooklyn-client pull request #44: Added the ability to upload a zip via the ...
GitHub user Graeme-Miller opened a pull request:
https://github.com/apache/brooklyn-client/pull/44
Added the ability to upload a zip via the CLI
@geomacy would you please review?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Graeme-Miller/brooklyn-client upload_zip
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-client/pull/44.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #44
----
commit 1cf07d53241c80e1552b8af224f634ce5c7f8170
Author: graeme.miller <gr...@cloudsoftcorp.com>
Date: 2017-04-05T15:12:04Z
Added the ability to upload a zip via the CLI
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #44: Added the ability to upload a zip via the CLI
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-client/pull/44
confirmed that file URLs do not seem to work. additionally i tried the following `https` url:
`target/br catalog add 'https://doc-08-bs-docs.googleusercontent.com/docs/securesc/ha0ro937gcuc7l7deffksulhg5h7mbp1/gvfvaob3c31nd2c4sq8j2d7rra0uogbf/1493028000000/05557496147892979345/*/0B3XurVLRa7pIWnhRV0swVktreUU?e=download'`
and got:
`ReaderException: special characters are not allowed`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #44: Added the ability to upload a zip via the ...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/44#discussion_r110366235
--- Diff: cli/api/catalog/catalog.go ---
@@ -170,15 +178,102 @@ func Locations(network *net.Network) ([]models.CatalogItemSummary, error) {
return catalogLocations, err
}
+
+func ZipResource(resource string) (*bytes.Buffer, error) {
+ buf := new(bytes.Buffer)
+ writer := zip.NewWriter(buf)
+ defer writer.Close()
+
+ walkFn := func(path string, info os.FileInfo, err error) error {
+ if info.IsDir() {
+ return nil
+ }
+
+ relativePath, err := filepath.Rel(resource, path)
+ if err != nil {
+ return err
+ }
+ f, err := writer.Create(relativePath)
+ if err != nil {
+ return err
+ }
+
+ fileBytes, err := ioutil.ReadFile(path)
+ if err != nil {
+ return err
+ }
+
+ _, err = f.Write(fileBytes)
+ if err != nil {
+ return err
+ }
+ return nil
+ }
+
+ err := filepath.Walk(resource, walkFn)
+
+ return buf, err;
+}
+
func AddCatalog(network *net.Network, resource string) (map[string]models.CatalogEntitySummary, error) {
- url := "/v1/catalog"
+ urlString := "/v1/catalog"
var entities map[string]models.CatalogEntitySummary
- body, err := network.SendPostResourceRequest(url, resource, "application/json")
+
+ //Assume application/json. This is correct for http/file resources.
+ //Zips will need application/x-zip
+ contentType := "application/json"
+ u, err := url.Parse(resource)
+ if err != nil {
+ return nil, err
+ }
+
+ //Only deal with the below file types
+ if "" != u.Scheme && "file" != u.Scheme && "http" != u.Scheme && "https" != u.Scheme{
+ return nil, errors.New("Unrecognised protocol scheme: " + u.Scheme)
--- End diff --
"Unsupported" maybe
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #44: Added the ability to upload a zip via the CLI
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-client/pull/44
also noted that:
```
% target/br catalog add
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x12c9794]
goroutine 1 [running]:
github.com/apache/brooklyn-client/cli/api/catalog.ZipResource.func1(0x0, 0x0, 0x0, 0x0, 0x150a540, 0xc420011860, 0x20, 0x1355c80)
/Users/alex/go/src/github.com/apache/brooklyn-client/cli/api/catalog/catalog.go:188 +0x44
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #44: Added the ability to upload a zip via the CLI
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-client/pull/44
I also noted that any update that doesn't change a version is silently ignored, no error and no effect. desired behaviour is that:
* if the update is a snapshot, simply replace it in the catalog
* if the update is not a snapshot, generate an error
This is NOT to do with this PR; it needs a change in the brooklyn-server REST API behaviour -- but I only noticed it here. (I was running with https://github.com/apache/brooklyn-server/pull/645 when observed but I don't think that is significant.)
The idea is that a workflow where you make a change to a BOM or script, then `br catalog add` and good things happen if you're on a snapshot version -- do another `br deploy` and you should get the latest version. (And ideally we could then add an `--update-all-deployments` to `catalog add` and deployments would be updated!)
Already with this PR it is much easier you just have to remember to bump the version number but we can do better.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #44: Added the ability to upload a zip via the CLI
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-client/pull/44
working on fixing the items noted above -- in https://github.com/ahgittin/brooklyn-client/commit/6f6e73655af4b51abf2eaa747d3f343673e5e7a8
will open a new PR including that
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #44: Added the ability to upload a zip via the CLI
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-client/pull/44
@Graeme-Miller @geomacy To summarise above comments, I think it is important that we:
* ensure `file:` URLs work, at least for files (directories optional)
* ensure `http` and `https` URLs work, at least for files (directories impossible to guarantee; heuristics might be nice but not worth it), including the link above which gave the `ReaderException`
* update snapshot items in catalog (separate PR obviously)
and would be nice to:
* display nice error message if incomplete input supplied
We could merge this PR as is if there is not immediate bandwidth as the feature is already _so_ useful -- I just wouldn't want the observed issues to get forgotten.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #44: Added the ability to upload a zip via the CLI
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-client/pull/44
I've merged this, as it's useful, and opened PR #48 fixing the other items noted. It addresses everything Geoff and I listed, except for the snapshot-not-updating problem which is server side (and I'll look at next).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #44: Added the ability to upload a zip via the CLI
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-client/pull/44
Code changes look good; but problems with test:
I tested this out as follows:
- upload a zip file as `br catalog add test.zip` - success
- upload a directory as `br catalog add test4` - success
- upload a zip file via "file" URL: `br catalog add file:///Users/geoff/tmp/test3/test.zip` - fails, with
open file:/Users/geoff/tmp/test3/test.zip: no such file or directory
From looking at the code I thought this should work, is this how it's intended?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #44: Added the ability to upload a zip via the ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-client/pull/44
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request #44: Added the ability to upload a zip via the ...
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/44#discussion_r111962100
--- Diff: cli/api/catalog/catalog.go ---
@@ -23,6 +23,14 @@ import (
"fmt"
"github.com/apache/brooklyn-client/cli/models"
"github.com/apache/brooklyn-client/cli/net"
+ "net/url"
+ "path/filepath"
--- End diff --
indentation, worth adding `golint` and `go vet` to the build process to pick up stuff like this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client issue #44: Added the ability to upload a zip via the CLI
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on the issue:
https://github.com/apache/brooklyn-client/pull/44
@geomacy can you review now you are back? cheers
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---