Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cdns/{{name}}/federations and cdns/{{name}}/federations/{{ID}} use RFC3339 timestamps #7806

Merged
merged 20 commits into from
Sep 20, 2023

Conversation

ocket8888
Copy link
Contributor

Fixes #7799 and thereby closes #5911

This PR updates the "CDN Federations" endpoint(s) to use RFC3339 timestamps. Also, with the recent sphinx upgrade, our RFC text roles aren't working due to missing grave accents, so I fixed those.


Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Ops Client (Go)
  • Traffic Ops

What is the best way to verify this PR?

Make sure all of the tests provided pass. I added a bunch of unit tests, too - not nearly 100% coverage, but better than nothing.

PR submission checklist

  • This PR has tests
  • This PR has documentation
  • This PR has a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added Traffic Ops related to Traffic Ops documentation related to documentation low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution TO Client (Go) related to the Go implementation of a TC client labels Sep 15, 2023
@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #7806 (e569f84) into master (0068dd8) will increase coverage by 3.02%.
The diff coverage is 45.17%.

@@             Coverage Diff              @@
##             master    #7806      +/-   ##
============================================
+ Coverage     28.83%   31.86%   +3.02%     
  Complexity       98       98              
============================================
  Files           600      717     +117     
  Lines         77175    82720    +5545     
  Branches         90      965     +875     
============================================
+ Hits          22256    26356    +4100     
- Misses        52832    54203    +1371     
- Partials       2087     2161      +74     
Flag Coverage Δ
golib_unit 53.56% <ø> (ø)
grove_unit 12.02% <ø> (ø)
t3c_unit 5.91% <ø> (ø)
traffic_monitor_unit 26.44% <ø> (ø)
traffic_ops_integration 69.42% <100.00%> (ø)
traffic_ops_unit 21.67% <43.42%> (+0.16%) ⬆️
traffic_portal_v2 74.38% <ø> (?)
traffic_stats_unit 10.78% <ø> (ø)
unit_tests 29.18% <43.42%> (+3.53%) ⬆️
v3 57.79% <ø> (ø)
v4 79.18% <ø> (ø)
v5 78.58% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
lib/go-tc/federation.go 0.00% <ø> (ø)
traffic_ops/traffic_ops_golang/server/servers.go 27.36% <0.00%> (+0.04%) ⬆️
traffic_ops/traffic_ops_golang/util/ims/ims.go 31.70% <0.00%> (ø)
...traffic_ops_golang/cdnfederation/cdnfederations.go 22.56% <42.43%> (+22.56%) ⬆️
traffic_ops/traffic_ops_golang/api/api.go 35.32% <100.00%> (+0.54%) ⬆️
traffic_ops/traffic_ops_golang/routing/routes.go 95.53% <100.00%> (+<0.01%) ⬆️
traffic_ops/v5-client/cdnfederations.go 100.00% <100.00%> (ø)

... and 120 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@rimashah25 rimashah25 left a comment

Choose a reason for hiding this comment

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

Code changes LGTM. Need to test on local.

@rimashah25
Copy link
Contributor

Need to add a TTL check like one sees on the UI.

@ocket8888 ocket8888 force-pushed the to/cdn-federations-rfc3339 branch 2 times, most recently from 956b750 to d5c4ebb Compare September 20, 2023 01:57
@ocket8888 ocket8888 force-pushed the to/cdn-federations-rfc3339 branch from d5c4ebb to e569f84 Compare September 20, 2023 15:28
@rimashah25
Copy link
Contributor

Test look good. Approved.

@rimashah25 rimashah25 merged commit de900da into apache:master Sep 20, 2023
rimashah25 added a commit to rimashah25/trafficcontrol that referenced this pull request Dec 4, 2023
…pache#7718 (apache#49)

* Updated TP field names based on TO changes from PRs apache#7806, apache#7718,

* Updated TP field name (cdn) in server capability and updated changelog
rimashah25 added a commit to rimashah25/trafficcontrol that referenced this pull request Dec 4, 2023
…pache#7718

Updated TP field name (cdn) in server capability and updated changelog
rimashah25 added a commit to rimashah25/trafficcontrol that referenced this pull request Dec 4, 2023
…pache#7718

Updated TP field name (cdn) in server capability and updated changelog
zrhoffman pushed a commit that referenced this pull request Dec 4, 2023
* * Fixed broken capability links for DS

* Updated CHANGELOG.md

* Updated based on review comment as well as removed deleteServerCapability button(DS table) and menu-option(right click)

* Updated TP field names based on TO changes from ATC PRs #7806, #7718

Updated TP field name (cdn) in server capability and updated changelog

* Updated broken links in DS's right click menu
rimashah25 added a commit that referenced this pull request Jan 3, 2024
Updated based on review comment as well as removed deleteServerCapability button(DS table) and menu-option(right click)
Updated TP field names based on TO changes from ATC PRs #7806, #7718
Updated TP field name (cdn) in server capability and updated changelog
Updated broken links in DS's right click menu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation related to documentation low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution TO Client (Go) related to the Go implementation of a TC client Traffic Ops related to Traffic Ops
Projects
None yet
3 participants