add Traffic fields to the Service Schema #9953
Conversation
knative-prow-robot
added
the
size/M
Codecov Report
@@ Coverage Diff @@
## master #9953 +/- ##
==========================================
+ Coverage 87.81% 87.83% +0.02%
==========================================
Files 183 183
Lines 8631 8631
==========================================
+ Hits 7579 7581 +2
+ Misses 801 800 -1
+ Partials 251 250 -1
Continue to review full report at Codecov.
|
|
/assign @dprotaso |
|
/retest |
| from the prior "latest ready" revision to the new one. This field is never | ||
| set in Route's status, only its spec. | ||
| This is mutually exclusive with RevisionName. | ||
| +optional |
There was a problem hiding this comment.
Does it make sense to drop these +optional?
There was a problem hiding this comment.
It was my understanding that you could have only one of revisionName OR configurationName. Unless you are saying that configurationName can never be in traffic.
I copied the comments from the API.
OR are you just saying that it may be too much api specific info?
There was a problem hiding this comment.
I was referring to having the literal +optional in the description. The prior text seems clear enough
There was a problem hiding this comment.
I can remove it.
| Scheme: | ||
| type: string | ||
| Opaque: | ||
| type: string | ||
| description: encoded opaque data | ||
| User: | ||
| type: object | ||
| description: username and password information | ||
| properties: | ||
| username: | ||
| type: string | ||
| password: | ||
| type: string | ||
| passwordSet: | ||
| type: boolean | ||
| Host: | ||
| type: string | ||
| description: host or host:port | ||
| Path: | ||
| type: string | ||
| description: path (relative paths may omit leading slash) | ||
| RawPath: | ||
| type: string | ||
| description: encoded path hint (see EscapedPath method) | ||
| ForceQuery: | ||
| type: boolean | ||
| description: append a query ('?') even if RawQuery is empty | ||
| RawQuery: | ||
| type: string | ||
| description: encoded query values, without '?' | ||
| Fragment: | ||
| type: string | ||
| description: fragment for references, without '#' |
There was a problem hiding this comment.
This isn't accurate. You'll have to look at how the struct is serialized.
There was a problem hiding this comment.
Actually the url property I believe is only present in the object's status block so I believe this whole thing can be removed.
There was a problem hiding this comment.
The struct for TrafficTarget has
URL *apis.URL json:"url,omitempty"
I used the fields for url.URL.
Can you point me to where it is serialized. I'm confused if the type is URL and the schema has the details of URL, what is wrong. I'm not disagreeing, I'm just confused.
There was a problem hiding this comment.
Just saw the comment to remove it. I can do that if you want.
There was a problem hiding this comment.
We should be able to remove it from the spec
There was a problem hiding this comment.
ok, I'll remove it
joshuawilson
force-pushed
the
service-schema
branch
from
c82f490
to
e72fd5f
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprotaso, joshuawilson The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
Related to #912
Proposed Changes
Release Note