Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Add Go implementation of GET, POST federations/{id}/federation_resolvers#4191

Merged
mitchell852 merged 20 commits into
apache:masterfrom
lactose:dev/rewrite-federations-federation-resolvers
Dec 30, 2019
Merged

Add Go implementation of GET, POST federations/{id}/federation_resolvers#4191
mitchell852 merged 20 commits into
apache:masterfrom
lactose:dev/rewrite-federations-federation-resolvers

Conversation

@lactose

@lactose lactose commented Dec 9, 2019

Copy link
Copy Markdown
Contributor

What does this PR (Pull Request) do?

This is a rewrite of the perl endpoints GET,POST for /federations/{id}/federation_resolvers

Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

Run the added API test against both the Perl and Go backends.

You can also test this in TP by adding federation resolvers to federations and ensuring that they show up in the UI.

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct OR this PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

@lactose lactose changed the title Dev/rewrite federations federation resolvers Add Go implementation of GET, POST federations/{id}/federation_resolvers Dec 9, 2019
@asf-ci

asf-ci commented Dec 9, 2019

Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@mitchell852 mitchell852 added Traffic Ops related to Traffic Ops tech debt rework due to choosing easy/limited solution labels Dec 10, 2019
@mitchell852 mitchell852 added this to the Go Rewrite milestone Dec 10, 2019
@lactose
lactose force-pushed the dev/rewrite-federations-federation-resolvers branch from ad91579 to bd1714a Compare December 18, 2019 17:34
Comment thread lib/go-tc/federation_federation_resolvers.go Outdated
Comment thread traffic_ops/testing/api/v14/cdnfederations_test.go Outdated
Comment thread traffic_ops/testing/api/v14/cdnfederations_test.go Outdated
Comment thread traffic_ops/testing/api/v14/cdnfederations_test.go Outdated
Comment thread traffic_ops/testing/api/v14/cdnfederations_test.go Outdated
Comment thread traffic_ops/traffic_ops_golang/federations/federation_resolvers.go Outdated
Comment thread traffic_ops/traffic_ops_golang/federations/federation_resolvers.go Outdated
Comment thread traffic_ops/traffic_ops_golang/federations/federation_resolvers.go Outdated
Comment thread traffic_ops/traffic_ops_golang/federations/federation_resolvers.go Outdated
Comment thread traffic_ops/traffic_ops_golang/federations/federations.go Outdated
Comment thread .golangci.yml Outdated
Comment thread traffic_ops/testing/api/v14/cdnfederations_test.go Outdated
Comment thread traffic_ops/traffic_ops_golang/federations/federation_resolvers.go Outdated
@mitchell852

Copy link
Copy Markdown
Member

@lactose - i'm seeing a couple of failing API tests:

=== RUN   TestCDNFederations/successfully_get_federation_federation_resolvers_for_a_federation_with_some
    --- FAIL: TestCDNFederations/successfully_get_federation_federation_resolvers_for_a_federation_with_some (0.02s)
        cdnfederations_test.go:222: expected federation of ID 19 to have associated federation resolvers, but had 0
=== RUN   TestFederationFederationResolvers/Fail_to_assign_federation_resolver_to_a_federation_when_federation_does_not_exist
    --- FAIL: TestFederationFederationResolvers/Fail_to_assign_federation_resolver_to_a_federation_when_federation_does_not_exist (0.01s)
        cdnfederations_test.go:186: error: expected error result 404 Not Found[404] - Error requesting Traffic Ops https://localhost:8443/api/1.4/federations/-1/federation_resolvers {"alerts":[{"text":"'-1': no such Federation","level":"error"}]}
            , want: Resource not found

@lactose
lactose force-pushed the dev/rewrite-federations-federation-resolvers branch from 426215d to 25f0e5c Compare December 24, 2019 23:27
@lactose

lactose commented Dec 25, 2019

Copy link
Copy Markdown
Contributor Author

@mitchell852 tests are now working

@mitchell852

mitchell852 commented Dec 26, 2019

Copy link
Copy Markdown
Member

@lactose - i'm seeing a difference between the Perl's GET behavior and the Go's GET behavior:

Perl:

GET api/1.4/federations/1/federation_resolvers where no fed resolvers are assigned to the federation:

{"response":[]}

Go:

GET api/1.4/federations/1/federation_resolvers where no fed resolvers are assigned to the federation:

{"response":null}

The POST, however, looks fine.

@lactose

lactose commented Dec 30, 2019

Copy link
Copy Markdown
Contributor Author

@mitchell852 pushed an update

@mitchell852 mitchell852 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like all the PR comments have been addressed and the behavior emulates that of the Perl endpoints. All unit tests and API tests pass as well as well as a manual test in TP.

@mitchell852
mitchell852 merged commit 95a6280 into apache:master Dec 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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.

Rewrite /federations/{{id}}/federation_resolvers to Go

4 participants