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

fix to ensure to call requestdone() when requestkey is valid #1558

Conversation

sykim-etri
Copy link
Member

@sykim-etri sykim-etri commented May 8, 2024

CB-SP에서 에러를 리턴하는 경우도 왕왕(동일한 이름의 노드그룹 추가 등) 있기 때문에 requestkey 값을 확인한 후 requestDone()을 호출하도록 수정한 PR입니다.
method가 GET인 경우에 한정하여 requestDone()을 호출하는 방안도 있겠습니다.
의견 주시면 반영하도록 하겠습니다.

fix: #1557

@seokho-son
Copy link
Member

@sykim-etri 버그 확인 및 개선 감사합니다. :)

해당 사항은

func requestDone(requestKey string) {

함수 내에서 개선하면 좋을 것 같습니다.

  • if requestKey != "" 인 경우 함수 내에서 바로 리턴하고,
  • count, _ := clientRequestCounter.Load(requestKey) 에 err 에러 핸들링 추가해서, err이 있는 경우 그냥 리턴하면 좋을 것 같습니다.

어떠신지요?

@sykim-etri
Copy link
Member Author

@sykim-etri 버그 확인 및 개선 감사합니다. :)

해당 사항은

func requestDone(requestKey string) {

함수 내에서 개선하면 좋을 것 같습니다.

  • if requestKey != "" 인 경우 함수 내에서 바로 리턴하고,
  • count, _ := clientRequestCounter.Load(requestKey) 에 err 에러 핸들링 추가해서, err이 있는 경우 그냥 리턴하면 좋을 것 같습니다.

어떠신지요?

방어 코드를 추가하는 측면에서는 바람직할 것 같습니다.

requestKey 생성 시점과의 pair를 생각한다면.. GET인 경우만 requestDone을 호출하도록 수정하고.. 의견주신 방어코드를 추가하는 것은 어떨까요?

@seokho-son
Copy link
Member

@sykim-etri 넵! 제안해주신 방식이 더 명확할 것 같습니다. :)

@sykim-etri sykim-etri force-pushed the fix-to-call-requestdone-when-requestkey-is-valid branch from 1ac7e6a to afd7fdb Compare May 8, 2024 09:23
@seokho-son
Copy link
Member

/approve

@github-actions github-actions bot added the approved This PR is approved and will be merged soon. label May 8, 2024
@cb-github-robot cb-github-robot merged commit 3926e2d into cloud-barista:main May 8, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved and will be merged soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to debug this panic, that is "[PANIC RECOVER] interface conversion: interface {} is nil, not int"?
3 participants