You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/12/06 19:46:17 UTC
[GitHub] [trafficcontrol] ocket8888 opened a new pull request, #7235: Fix APIv4 and under DSRs not having Original set on Update DSRs
ocket8888 opened a new pull request, #7235:
URL: https://github.com/apache/trafficcontrol/pull/7235
DSRs in versions earlier than 5.0 should have their requested _and_ original DS's set, but because of a bug in the downgrade process, the original was being discarded. This PR fixes that bug.
<hr/>
## Which Traffic Control components are affected by this PR?
- Traffic Ops
## What is the best way to verify this PR?
I added unit test coverage for the case where neither original nor requested should be nil, so it should suffice that the tests pass. But the way this bug was found was because the DSR edit page was busted, so you may want to check that out to make sure it works.
## If this is a bugfix, which Traffic Control versions contained the bug?
- master
## PR submission checklist
- [x] This PR has tests
- [x] This PR doesn't need documentation
- [x] This PR doesn't need a CHANGELOG.md entry
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] codecov[bot] commented on pull request #7235: Fix APIv4 and under DSRs not having Original set on Update DSRs
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #7235:
URL: https://github.com/apache/trafficcontrol/pull/7235#issuecomment-1339922932
# [Codecov](https://codecov.io/gh/apache/trafficcontrol/pull/7235?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7235](https://codecov.io/gh/apache/trafficcontrol/pull/7235?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5bdfefd) into [master](https://codecov.io/gh/apache/trafficcontrol/commit/9bb5ad527b9c3e3dfcce86cd9bab478431d3e1d2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9bb5ad5) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #7235 +/- ##
============================================
+ Coverage 28.36% 28.37% +0.01%
Complexity 98 98
============================================
Files 617 617
Lines 69195 69197 +2
Branches 90 90
============================================
+ Hits 19624 19633 +9
+ Misses 47760 47755 -5
+ Partials 1811 1809 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| golib_unit | `53.07% <100.00%> (+0.07%)` | :arrow_up: |
| grove_unit | `4.60% <ø> (ø)` | |
| t3c_generate_unit | `24.96% <ø> (ø)` | |
| traffic_monitor_unit | `20.43% <ø> (ø)` | |
| traffic_stats_unit | `10.41% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/trafficcontrol/pull/7235?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [lib/go-tc/deliveryservice\_requests.go](https://codecov.io/gh/apache/trafficcontrol/pull/7235/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliL2dvLXRjL2RlbGl2ZXJ5c2VydmljZV9yZXF1ZXN0cy5nbw==) | `44.95% <100.00%> (+2.00%)` | :arrow_up: |
| [lib/go-atscfg/parentabstraction.go](https://codecov.io/gh/apache/trafficcontrol/pull/7235/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliL2dvLWF0c2NmZy9wYXJlbnRhYnN0cmFjdGlvbi5nbw==) | `79.47% <0.00%> (-0.53%)` | :arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on pull request #7235: Fix APIv4 and under DSRs not having Original set on Update DSRs
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on PR #7235:
URL: https://github.com/apache/trafficcontrol/pull/7235#issuecomment-1341707725
Tested on local and now I do see the current value populated correctly with old value and new value is seen in the field.
<img width="726" alt="image" src="https://user-images.githubusercontent.com/22248619/206314902-820b55de-7a81-49fd-bb93-f8e05d4bc555.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on pull request #7235: Fix APIv4 and under DSRs not having Original set on Update DSRs
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on PR #7235:
URL: https://github.com/apache/trafficcontrol/pull/7235#issuecomment-1343009472
> Also while your changes technically catches the error, I still think those should be fixed (or at least not pollute the console). Just changing the Content Routing Type causes a dozen of the label not found error.
Changing anything will do that, just because it triggers a digest cycle. All of those errors are coming from DSCP labeling, because many DSCPs aren't labeled. Tell me, though, what does a DSCP of 10 mean? According to the "human-friendly" label, it means "10 - AF11". What's that? Our docs don't tell you, there's no tooltip that tells you. Even better, look at a DSCP of 37. What that value means - according to the label - is simply "37 - ". Insightful.
I think these are proprietary labels that have no real meaning for the ATC project, and IMO we'd be better off without them. Numbers is numbers. That would stop the log spam. But that's also unrelated to this PR, could be done at a later date if we don't think it's safe to pull that trigger right now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on pull request #7235: Fix APIv4 and under DSRs not having Original set on Update DSRs
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on PR #7235:
URL: https://github.com/apache/trafficcontrol/pull/7235#issuecomment-1341501702
Code LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus merged pull request #7235: Fix APIv4 and under DSRs not having Original set on Update DSRs
Posted by GitBox <gi...@apache.org>.
shamrickus merged PR #7235:
URL: https://github.com/apache/trafficcontrol/pull/7235
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org