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

Use RFC 3339 for lastUpdated timestamp in /server_server_capabilities #7744

Merged
merged 2 commits into from
Aug 28, 2023

Conversation

zrhoffman
Copy link
Member

This PR resolves #7743 by using the timestamp format specified in RFC 3339 for the lastUpdated field of the /api/5.0/server_server_capabilities endpoint.


Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

Run the server_server_capabilities API tests

PR submission checklist

@zrhoffman zrhoffman added Traffic Ops related to Traffic Ops low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution improvement The functionality exists but it could be improved in some way. labels Aug 25, 2023
@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Merging #7744 (c2e7652) into master (75ec56f) will decrease coverage by 32.98%.
Report is 91 commits behind head on master.
The diff coverage is 60.64%.

@@              Coverage Diff              @@
##             master    #7744       +/-   ##
=============================================
- Coverage     65.05%   32.07%   -32.98%     
  Complexity       98       98               
=============================================
  Files           314      712      +398     
  Lines         12365    81475    +69110     
  Branches        907      965       +58     
=============================================
+ Hits           8044    26134    +18090     
- Misses         3968    53195    +49227     
- Partials        353     2146     +1793     
Flag Coverage Δ
golib_unit 53.70% <60.29%> (?)
grove_unit 12.02% <ø> (?)
t3c_unit 5.99% <3.31%> (?)
traffic_monitor_unit 26.33% <0.00%> (?)
traffic_ops_integration 69.38% <ø> (-0.04%) ⬇️
traffic_ops_unit 21.87% <ø> (?)
traffic_portal_v2 74.38% <74.17%> (+0.59%) ⬆️
traffic_stats_unit 10.76% <ø> (?)
unit_tests 29.38% <60.64%> (-44.40%) ⬇️
v3 57.79% <ø> (ø)
v4 79.18% <ø> (ø)
v5 78.49% <ø> (-0.14%) ⬇️

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

Files Changed Coverage Δ
cache-config/t3c-apply/config/config.go 0.00% <0.00%> (ø)
cache-config/t3c-apply/torequest/cmd.go 0.00% <0.00%> (ø)
cache-config/t3c-check-refs/t3c-check-refs.go 1.78% <0.00%> (ø)
cache-config/t3c-generate/cfgfile/varnish.go 0.00% <0.00%> (ø)
cache-config/t3c-generate/config/config.go 0.81% <0.00%> (-0.08%) ⬇️
cache-config/t3c-generate/t3c-generate.go 0.00% <ø> (ø)
experimental/traffic-portal/src/app/api/index.ts 100.00% <ø> (ø)
...mental/traffic-portal/src/app/api/testing/index.ts 100.00% <ø> (ø)
...perimental/traffic-portal/src/app/app.ui.module.ts 100.00% <ø> (ø)
...tatuses/statuses-table/statuses-table.component.ts 82.35% <ø> (ø)
... and 86 more

... and 367 files with indirect coverage changes

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

Copy link
Contributor

@kdamichie kdamichie left a comment

Choose a reason for hiding this comment

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

Check #1 - simple changes requested

CHANGELOG.md Outdated Show resolved Hide resolved
traffic_ops/v5-client/server_server_capabilities.go Outdated Show resolved Hide resolved
func (to *Session) GetServerServerCapabilities(opts RequestOptions) (tc.ServerServerCapabilitiesResponse, toclientlib.ReqInf, error) {
var resp tc.ServerServerCapabilitiesResponse
func (to *Session) GetServerServerCapabilities(opts RequestOptions) (tc.ServerServerCapabilitiesResponseV5, toclientlib.ReqInf, error) {
var resp tc.ServerServerCapabilitiesResponseV5
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: GetServerServerCapabilities should probably be GetServerServerCapabilitiesV5

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't want V5 in the struct method name, since this is already the v5 version of the Session struct

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good

Copy link
Contributor

Choose a reason for hiding this comment

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

Verified v5 servers_server_capability_test.go DOES test the new code.

Copy link
Contributor

@kdamichie kdamichie left a comment

Choose a reason for hiding this comment

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

Approved!

@zrhoffman zrhoffman merged commit 62bd082 into apache:master Aug 28, 2023
@zrhoffman zrhoffman deleted the rfc-3339-ssc branch August 28, 2023 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement The functionality exists but it could be improved in some way. low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution Traffic Ops related to Traffic Ops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

/server_server_capabilities in TO API uses non-RFC3339 date/time strings
2 participants