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

[DevOps] .spectral.yaml 수정 - 404, 400 정의 #268 #305

Merged
merged 3 commits into from
Sep 7, 2024

Conversation

o-ii
Copy link
Contributor

@o-ii o-ii commented Sep 7, 2024

Issue

#268

[DevOps] .spectral.yaml 수정 - 404, 400 정의

  • 404 린팅 조건: path 에 path variable 있다면 404 응답 코드 정의
  • 400 린팅 조건: verb가 POST, PUT, PATCH일 경우 400 응답코드 정의

Description

.spectral.yaml 파일에 404, 400 상태 코드에 대한 린팅 규칙을 추가했습니다.

Summary of Changes

1. 404 린팅 조건

  • path(경로)에 path variable(경로 변수)가 포함되어 있을 경우, 404 응답 코드가 있어야 함
  # Ensure endpoints with path variables define a 404 response
  # path에 path variable이 있다면 404 응답 코드가 있어야 함
  path-variables-require-404:
    description: Path variables must include a 404 response
    severity: error
    given: $.paths[*][*].parameters[?(@.in == 'path')]^^
    then:
      - field: responses.404
        function: truthy
        message: Response 404 is required

^ for grabbing the parent of a matching item

린팅 테스트 결과

  • POST /openai/deployments/{deploymentName}/chat/completions
//-----------------------------------------------------------------------
// src/AzureOpenAIProxy.ApiApp/Endpoints/ProxyChatCompletionsEndpoint.cs
//-----------------------------------------------------------------------

// 수정 전 (error 발생)

var builder = app.MapPost(ProxyEndpointUrls.ChatCompletions, async () =>
            //( 생략 ... )
        })
        .Produces(statusCode: StatusCodes.Status400BadRequest)
        .Produces(statusCode: StatusCodes.Status401Unauthorized)
        //.Produces(statusCode: StatusCodes.Status404NotFound)
Screenshot 2024-09-07 at 12 16 10 PM
// 수정 후 (error 발생한 엔드포인트에 400 응답코드 추가)

var builder = app.MapPost(ProxyEndpointUrls.ChatCompletions, async () =>
            //( 생략 ... )
        })
        .Produces(statusCode: StatusCodes.Status400BadRequest)
        .Produces(statusCode: StatusCodes.Status401Unauthorized)
        .Produces(statusCode: StatusCodes.Status404NotFound)
Screenshot 2024-09-07 at 12 25 33 PM

2. 400 린팅 조건

  • POST, PUT, PATCH 메서드를 사용하는 경우, 400 응답코드가 있어야 함
  # Ensure POST, PUT, PATCH methods define a 400 response
  # verb가 POST, PUT, PATCH일 경우 400 응답코드가 있어야 함
  post-put-patch-require-400:
    description: POST, PUT, PATCH methods must include a 400 response
    given: $.paths[*][?(@property == 'post' || @property == 'put' || @property == 'patch')]
    severity: error
    then:
      - field: responses.400
        function: truthy
        message: Response 400 is required

린팅 테스트 결과

  • POST /admin/events
//--------------------------------------------------------------
// src/AzureOpenAIProxy.ApiApp/Endpoints/AdminEventEndpoints.cs
//--------------------------------------------------------------

// 수정 전 (error 발생)

var builder = app.MapPost(AdminEndpointUrls.AdminEvents, async () =>
            //( 생략 ... )
        })
        //.Produces(statusCode: StatusCodes.Status400BadRequest)
        .Produces(statusCode: StatusCodes.Status401Unauthorized)
Screenshot 2024-09-07 at 2 45 08 PM
// 수정 후 (error 발생한 엔드포인트에 400 응답코드 추가)

var builder = app.MapPost(AdminEndpointUrls.AdminEvents, async () =>
            //( 생략 ... )
        })
        .Produces(statusCode: StatusCodes.Status400BadRequest)
        .Produces(statusCode: StatusCodes.Status401Unauthorized)
Screenshot 2024-09-07 at 12 25 33 PM

How to Test (from #229)

# .NET 프로젝트를 백그라운드에서 실행
dotnet run --project src/AzureOpenAIProxy.AppHost &

# .NET 프로젝트의 PID를 저장
DOTNET_PID=$!

# .NET 프로젝트가 완전히 기동될 때까지 대기 (필요에 따라 조정)
sleep 10

# Swagger 파일 다운로드
rm -f swagger.json
curl https://localhost:7001/swagger/v1.0.0/swagger.json -O

# .NET 프로젝트 종료
kill $DOTNET_PID

# Spectral로 린트 수행
spectral lint swagger.json

Reference

@tae0y
Copy link
Contributor

tae0y commented Sep 7, 2024

같이 도전해보아욥

Copy link
Contributor

@tae0y tae0y left a comment

Choose a reason for hiding this comment

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

@o-ii
src/AzureOpenAIProxy.ApiApp/Endpoints/WeatherForecastEndpoint.cs
GET /weatherforecast 부분에서 404 린팅을,

src/AzureOpenAIProxy.ApiApp/Endpoints/AdminEventEndpoints.cs
POST /admin/events 부분에서 400 린팅을 테스트해볼 수 있겠네요

코드단에서 400, 404 부분을 추가, 삭제하며 테스트해보고
결과를 PR 본문에 첨부해주실래욥?

@o-ii o-ii requested a review from tae0y September 7, 2024 04:40
@o-ii
Copy link
Contributor Author

o-ii commented Sep 7, 2024

src/AzureOpenAIProxy.ApiApp/Endpoints/WeatherForecastEndpoint.cs
GET /weatherforecast 부분에서 404 린팅을,

src/AzureOpenAIProxy.ApiApp/Endpoints/AdminEventEndpoints.cs
POST /admin/events 부분에서 400 린팅을 테스트해볼 수 있겠네요

코드단에서 400, 404 부분을 추가, 삭제하며 테스트해보고 결과를 PR 본문에 첨부해주실래욥?

404 린팅은 POST /openai/deployments/{deploymentName}/chat/completions 부분에서, (path에 path variable 포함)
400 린팅은 POST /admin/events 부분에서
각각 테스트한 결과를 PR 본문에 추가했습니다.

Copy link
Contributor

@tae0y tae0y left a comment

Choose a reason for hiding this comment

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

src/AzureOpenAIProxy.ApiApp/Endpoints/WeatherForecastEndpoint.cs
GET /weatherforecast 부분에서 404 린팅을,
src/AzureOpenAIProxy.ApiApp/Endpoints/AdminEventEndpoints.cs
POST /admin/events 부분에서 400 린팅을 테스트해볼 수 있겠네요
코드단에서 400, 404 부분을 추가, 삭제하며 테스트해보고 결과를 PR 본문에 첨부해주실래욥?

404 린팅은 POST /openai/deployments/{deploymentName}/chat/completions 부분에서, (path에 path variable 포함) 400 린팅은 POST /admin/events 부분에서 각각 테스트한 결과를 PR 본문에 추가했습니다.

  • 테스트 케이스
테스트 케이스 설명 응답 정의
Path 변수에 404 응답 없음 Path 변수가 있지만 404 응답이 정의되지 않음 404 없음
Path 변수에 404 응답 있음 Path 변수가 있고 404 응답이 정의됨 404 있음
POST 요청에 400 응답 없음 POST 요청이지만 400 응답이 정의되지 않음 400 없음
POST 요청에 400 응답 있음 POST 요청이며 400 응답이 정의됨 400 있음
PUT 요청에 400 응답 없음 PUT 요청이지만 400 응답이 정의되지 않음 400 없음
PUT 요청에 400 응답 있음 PUT 요청이며 400 응답이 정의됨 400 있음
PATCH 요청에 400 응답 없음 PATCH 요청이지만 400 응답이 정의되지 않음 400 없음
PATCH 요청에 400 응답 있음 PATCH 요청이며 400 응답이 정의됨 400 있음

각각의 테스트 케이스에 대해 routerbuilder를 작성해보았고 (feat. copilot)
작성해주신 규칙이 모두 잘 적용되는 것을 확인했습니다 !

/Users/bachtaeyeong/10_SrcHub/azure-openai-sdk-proxy/swagger.json
 381:21  error  path-variables-require-404  (o-ii) Path variables must include a 404 response            paths./AddWeatherForecastPatchNo400/{id}.patch.responses
 381:21  error  post-put-patch-require-400  (o-ii) POST, PUT, PATCH methods must include a 400 response  paths./AddWeatherForecastPatchNo400/{id}.patch.responses
 437:21  error  path-variables-require-404  (o-ii) Path variables must include a 404 response            paths./AddWeatherForecastPatchWith400/{id}.patch.responses
 485:21  error  post-put-patch-require-400  (o-ii) POST, PUT, PATCH methods must include a 400 response  paths./AddWeatherForecastPostNo400.post.responses
 609:21  error  path-variables-require-404  (o-ii) Path variables must include a 404 response            paths./AddWeatherForecastPutNo400/{id}.put.responses
 609:21  error  post-put-patch-require-400  (o-ii) POST, PUT, PATCH methods must include a 400 response  paths./AddWeatherForecastPutNo400/{id}.put.responses
 665:21  error  path-variables-require-404  (o-ii) Path variables must include a 404 response            paths./AddWeatherForecastPutWith400/{id}.put.responses
 665:21  error  post-put-patch-require-400  (o-ii) POST, PUT, PATCH methods must include a 400 response  paths./AddWeatherForecastPutWith400/{id}.put.responses

@justinyoo justinyoo merged commit 068b0e5 into aliencube:main Sep 7, 2024
1 check passed
@o-ii o-ii deleted the feature/268-modify-spectral branch September 7, 2024 06:56
sikutisa pushed a commit to sikutisa/azure-openai-sdk-proxy that referenced this pull request Sep 7, 2024
sikutisa added a commit to sikutisa/azure-openai-sdk-proxy that referenced this pull request Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DevOps] .spectral.yaml 수정 - 404, 400 정의
3 participants