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

Fix Internal server error in update cachegroups by already exist cg name#6023

Closed
dmohan001c wants to merge 0 commit intoapache:masterfrom
dmohan001c:putcachegroup
Closed

Fix Internal server error in update cachegroups by already exist cg name#6023
dmohan001c wants to merge 0 commit intoapache:masterfrom
dmohan001c:putcachegroup

Conversation

@dmohan001c
Copy link
Contributor

@dmohan001c dmohan001c commented Jul 14, 2021

What does this PR (Pull Request) do?

This fixes the internal server error causing in update cache group with the already existing cache group name

Which Traffic Control components are affected by this PR?

  • CDN in a Box
  • Traffic Control Client
  • Traffic Ops
  • CI tests

What is the best way to verify this PR?

Execute all the Integration tests and make sure the tests are passed.

If this is a bug fix, what versions of Traffic Control are affected?

  • Master

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 DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

@mitchell852 mitchell852 added Traffic Ops related to Traffic Ops bug something isn't working as intended labels Jul 19, 2021
Copy link
Contributor

Choose a reason for hiding this comment

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

So since this might actually be a database error, this should still return an Internal Server Error in my opinion.
What you could do is add a check at the start of the Update method checking to see if the cachegroup with the given name(from the request body) has the same ID as that passed in the request. If true, then proceed, else, return a 400 or 409 status code with the correct error (something like cachegroup name should be unique).

Also, could you pls add some tests for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, the tc.DBError is now deprecated. You can just return err in its place, and nil in place of the user error.

@srijeet0406
Copy link
Contributor

This PR shows 0 files changed, was this a merge issue?

@srijeet0406
Copy link
Contributor

@dmohan001c Is there another PR that I need to review for this issue, since this one is closed?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug something isn't working as intended Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Internal Server Error for PUT Request on cachegroups/{{ID}} endpoint

3 participants