Skip to content

Add K8 Namespace into URI - #26

Open
catherinetcai wants to merge 3 commits into
masterfrom
task/cat/add-namespace
Open

Add K8 Namespace into URI#26
catherinetcai wants to merge 3 commits into
masterfrom
task/cat/add-namespace

Conversation

@catherinetcai

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread README.md
gURL's request format is as follows:
```bash
gurl -u <protocol|optional>://<k8-context|optional>/<host|kubernetes-service-name>:<port>/<service>/<rpc> -d '{ "field_name": "field_value" }'
gurl -u <protocol|optional>://<k8-context|optional if namespace not specified>/<k8-namespace|optional>/<host|kubernetes-service-name>:<port>/<service>/<rpc> -d '{ "field_name": "field_value" }'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is getting way too long, consider a way to break this up to be more readable.

Screen Shot 2019-07-09 at 11 27 12 AM

Comment thread pkg/util/uri.go
uriRegex = `((?P<protocol>[a-z0-9]{2,5})(?:\:\/\/)((?P<context>[0-9a-z._-]+)(?:\/))?)?(?P<host>[0-9a-z-_.]+)(?:\:)(?P<port>[0-9]{2,5})(?:\/)(?P<service>[0-9a-zA-Z._-]+)(?:\/)(?P<rpc>[0-9a-zA-Z._-]+)`
//
// K8 request with namespace:
// k8://sandbox-secure/kube-system/foo-service:50051/hello.world.package.Foo/Bar

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this regex is looking a bit unwieldy, is there not a more sensible way to parse this that doesn't require regex?

Comment thread pkg/util/uri.go
//
// K8 request with namespace:
// k8://sandbox-secure/kube-system/foo-service:50051/hello.world.package.Foo/Bar
uriRegex = `((?P<protocol>[a-z0-9]{2,5})(?:\:\/\/)((?P<context>[0-9a-z._-]+)(?:\/))?((?P<namespace>[0-9a-z._-]+)(?:\/))?)?(?P<host>[0-9a-z-_.]+)(?:\:)(?P<port>[0-9]{2,5})(?:\/)(?P<service>[0-9a-zA-Z._-]+)(?:\/)(?P<rpc>[0-9a-zA-Z._-]+)`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

😨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants