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

[Backend API] Implement endpoint for create event details #213 #281

Open
wants to merge 49 commits into
base: main
Choose a base branch
from

Conversation

tae0y
Copy link
Contributor

@tae0y tae0y commented Aug 31, 2024

진행상황을 공유하기 위한 PR 제출입니다.
#213 이슈 관련

  • 진행한 것
    • 명령어를 사용해 테이블 스토리지 생성 az storage account create -n tae0ymanulstorage -g rg-tae0y -l koreacentral --subscription ossca
    • 커넥션 스트링을 우선 appsettings에서 참조, 서비스는 Program에서 생성하여 의존성 주입
    • 데이터 생성시 파티션키, 로우키에 대한 고민
  • 하지 못한 것
    • 테이블 스토리지를 bicep/깃헙액션을 통해 배포하기 : 지금은 bicep에 추가했지만 azd up으로 배포되지 않음
    • ITableEntity 인터페이스를 구현하기
    • 테스트!

@justinyoo
Copy link
Contributor

  • 테이블 스토리지를 만들 때, 가장 먼저 하는 일이 테이블 컨테이너를 만드는 겁니다. 그리고 그다음에 고민해야 할 부분이 파티션 키예욥.
  • 테이블 컨테이너 이름은 뭘로 할까요? 모든 이벤트 관련 내용이 다 들어가니까 events가 나을 듯?
  • 파티션 키는 이벤트를 생성하는 거니까 event-details?
  • 로우키는 이벤트 ID 값으로 가정해도 될 듯 합니다.

이 문서 참조해 보세요: https://learn.microsoft.com/rest/api/storageservices/designing-a-scalable-partitioning-strategy-for-azure-table-storage

@justinyoo
Copy link
Contributor

필요하다면 새 태스크 이슈를 하나 생성해서 bicep 파일을 만들어 두는 것도 좋아요.

https://github.com/Azure-Samples/azd-starter-bicep/tree/main/infra/core/storage

우리 bicep 구조는 이걸 따라가고 있으므로 이 파일을 복사해서 넣으시구요, 테이블까지 하나 생성하는 형태로 수정하면 됩니다. 그리고 우리 infra/main.bicep 파일을 수정해서 연결하면 됩니다. 그러면 지금은 KeyVault 만 생성되는데, 앞으로는 Storage Account 까지 자동으로 만들어질 듯?

Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

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

코멘트 남겨뒀습니다

infra/aspire.bicep Outdated Show resolved Hide resolved
src/AzureOpenAIProxy.ApiApp/Program.cs Outdated Show resolved Hide resolved
@tae0y tae0y changed the title Feature/213 admin event create [Backend API] Implement endpoint for create event details #213 Aug 31, 2024
@justinyoo
Copy link
Contributor

@tae0y 서비스 레이어, 리포지토리 레이어 추가해 뒀습니다. 확인해 보시고, 나머지 로직을 추가해 보면 될 듯 해요.

@tae0y tae0y marked this pull request as draft September 1, 2024 05:47
@tae0y
Copy link
Contributor Author

tae0y commented Sep 1, 2024

@justinyoo 서비스 레이어, 리포지토리 레이어 병합했습니다!
검토 받을 준비가 되면 PR로 다시 전환해놓겠습니다.

@justinyoo
Copy link
Contributor

@tae0y 테스트가 깨지는데 확인 부탁해욥!

@tae0y
Copy link
Contributor Author

tae0y commented Sep 25, 2024

@tae0y 테스트가 깨지는데 확인 부탁해욥!

넵! 확인해보겠습니다
((테스트쪽은 아직 개발이 안끝났어욥ㅎㅎ))

@tae0y tae0y marked this pull request as ready for review September 25, 2024 14:04
@tae0y
Copy link
Contributor Author

tae0y commented Sep 25, 2024

테스트코드 작성완료했고,
로컬, Azure 배포중 오류가 나서 확인중입니다

Error: /home/runner/work/azure-openai-sdk-proxy/azure-openai-sdk-proxy/src/AzureOpenAIProxy.PlaygroundApp/Clients/OpenAIApiClient.cs(41,13): 
error CS0117: 'ChatCompletionOptions' does not contain a definition for 'MaxTokens' 
[/home/runner/work/azure-openai-sdk-proxy/azure-openai-sdk-proxy/src/AzureOpenAIProxy.PlaygroundApp/AzureOpenAIProxy.PlaygroundApp.csproj]

@tae0y
Copy link
Contributor Author

tae0y commented Sep 28, 2024

테스트코드 작성완료했고, 로컬, Azure 배포중 오류가 나서 확인중입니다

Error: /home/runner/work/azure-openai-sdk-proxy/azure-openai-sdk-proxy/src/AzureOpenAIProxy.PlaygroundApp/Clients/OpenAIApiClient.cs(41,13): 
error CS0117: 'ChatCompletionOptions' does not contain a definition for 'MaxTokens' 
[/home/runner/work/azure-openai-sdk-proxy/azure-openai-sdk-proxy/src/AzureOpenAIProxy.PlaygroundApp/AzureOpenAIProxy.PlaygroundApp.csproj]

Azure.AI.OpenAI 버전 올라가며 이슈가 있어서 아래 PR에 댓글 남겨두었습니다.
저희가 사용중인 필드가 2.0.6beta부터 deprecated 되었습니다.
#314 (comment)

@tae0y
Copy link
Contributor Author

tae0y commented Oct 2, 2024

테스트코드 작성완료했고, 로컬, Azure 배포중 오류가 나서 확인중입니다

Error: /home/runner/work/azure-openai-sdk-proxy/azure-openai-sdk-proxy/src/AzureOpenAIProxy.PlaygroundApp/Clients/OpenAIApiClient.cs(41,13): 
error CS0117: 'ChatCompletionOptions' does not contain a definition for 'MaxTokens' 
[/home/runner/work/azure-openai-sdk-proxy/azure-openai-sdk-proxy/src/AzureOpenAIProxy.PlaygroundApp/AzureOpenAIProxy.PlaygroundApp.csproj]

Azure.AI.OpenAI 버전 올라가며 이슈가 있어서 아래 PR에 댓글 남겨두었습니다. 저희가 사용중인 필드가 2.0.6beta부터 deprecated 되었습니다. #314 (comment)

@justinyoo
Hotfix(?) 적용후 다시 제출했습니다!
확인 부탁드립니다.

@@ -67,10 +67,33 @@ public void Given_Instance_When_CreateEvent_Invoked_Then_It_Should_Throw_Excepti
var repository = new AdminEventRepository(tableServiceClient, settings);

// Act
Func<Task> func = async () => await repository.CreateEvent(eventDetails);
var result = await repository.CreateEvent(eventDetails);
Copy link
Contributor

@justinyoo justinyoo Oct 3, 2024

Choose a reason for hiding this comment

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

이게 동작하나요????

https://learn.microsoft.com/en-us/dotnet/azure/sdk/unit-testing-mocking?tabs=nsubstitute

만약에 동작한다면, 그건 그것대로 문제 같은데... 왜냐면 목킹이 제대로 안 된 것 같아서;;;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오.....! ㅠㅠㅠ 보내주신 문서보고 다시 해보겠습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(수정내용)
단위 테스트 자체에 이해도가 낮아서, 아직 테스트 방법을 갈피를 잡지 못한 것 같습니다ㅠㅠ
우선 tableServiceClient가 tableClient를 반환하는 동작,
tableClient에서 호출되는 AddEntityAsync의 동작도 목킹을 추가했습니다.

(테스트목표)
이 테스트 메서드는 파라미터로 전달한 객체가 오류없이 잘 반환되는지,
그래서 파라미터로 전달한 것과 반환된 것이 동일한지 확인해보고 싶었어요.

(궁금한점)
음 저희 단위 테스트 코드를 보면 대체로 동작을 비슷한 방법으로 목킹하고 있는데
만약 어떠어떠한 부분이 잘 동작한다면, 이란 가정하에 나머지 부분이 어떻게 동작하는지를 확인하는 걸까요?

Copy link
Contributor

@justinyoo justinyoo Oct 4, 2024

Choose a reason for hiding this comment

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

(궁금한점)
음 저희 단위 테스트 코드를 보면 대체로 동작을 비슷한 방법으로 목킹하고 있는데 만약 어떠어떠한 부분이 잘 동작한다면, 이란 가정하에 나머지 부분이 어떻게 동작하는지를 확인하는 걸까요?

단위테스트는 내가 테스트하고자 하는 메소드가 제대로의도한대로 작동하는 것만을 확인합니다. 따라서, 그 안에서 어떤 일이 벌어지는지는 테스트하지 않아요.

예를 들어서 지금과 같이 CreateEvent라는 메소드를 보면, 그 안에서 TableClient를 만들잖아요? 근데, 이건 우리 메소드가 하는 코어 로직하고는 다르죠. 사실 어떻게 보면 이건 외부에서 인젝션해 주는 거나 마찬가지라서, 이 부분은 목킹으로 넘어가면 됩니다.

또한가지는 만약에 TableClient에서 우리가 AddEntity 메소드를 호출한다고 가정하면, 실제 상황에서는 이 메소드 호출시 에러가 나는 경우도 있겠죠? 그러면 AddEntity 메소드를 에러가 나는 상황으로 목킹해서 CreateEvent 메소드가 어떤식으로 에러 처리를 하는지 확인해야 할 겁니다. 또한 AddEntity 메소드를 에러가 나지 않는 상황으로 목킹해서 그 결과값을 테스트하면 됩니다.

정리하자면 우리가 테스트하려는 대상은 CreateEvent이고, 그 안에 들어가는 의존성 개체라든가 데이터는 가짜 데이터를 심어서 그 가짜 데이터를 바탕으로 우리 CreateEvent 메소드가 예상하는 대로 동작하는지를 확인하는 게 단위테스트의 목적입니다.

@tae0y
Copy link
Contributor Author

tae0y commented Oct 4, 2024

빌드 재시도하니 해결됨.


  • 로컬 빌드에서는 Given_ChatPrompt_When_Clear_Clicked_Then_ChatHistoryMessage_Should_Be_Cleared 테스트가 실패
지정된 패턴과 일치한 총 테스트 파일 수는 1개입니다.
  실패 Given_ChatPrompt_When_Clear_Clicked_Then_ChatHistoryMessage_Should_Be_Cleared("abcde") [1 s]
  오류 메시지:
   Expected result to be <null> or empty, but found "abcde".
  스택 추적:
     at FluentAssertions.Execution.LateBoundTestFramework.Throw(String message)
   at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message)
   at FluentAssertions.Execution.DefaultAssertionStrategy.HandleFailure(String message)
   at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc)
   at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc)
   at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args)
   at FluentAssertions.Primitives.StringAssertions`1.BeNullOrEmpty(String because, Object[] becauseArgs)
   at AzureOpenAIProxy.PlaygroundApp.Tests.Pages.PlaygroundPageTests.Given_ChatPrompt_When_Clear_Clicked_Then_ChatHistoryMessage_Should_Be_Cleared(String text) in /Users/bachtaeyeong/10_SrcHub/azure-openai-sdk-proxy/test/AzureOpenAIProxy.PlaygroundApp.Tests/Pages/PlaygroundPageChatWindowTests.cs:line 181
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.BlockUntilCompleted()
   at NUnit.Framework.Internal.MessagePumpStrategy.NoMessagePumpStrategy.WaitForCompletion(AwaitAdapter awaiter)
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0()
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)
  • 저희 팀 레포에서는 Given_Resource_When_Invoked_Endpoint_Then_It_Should_Return_Response 테스트가 실패
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:26.30]     AzureOpenAIProxy.AppHost.Tests.ApiApp.Endpoints.GetDeploymentModelsOpenApiTests.Given_Resource_When_Invoked_Endpoint_Then_It_Should_Return_Response(attribute: "401") [FAIL]
  Failed AzureOpenAIProxy.AppHost.Tests.ApiApp.Endpoints.GetDeploymentModelsOpenApiTests.Given_Resource_When_Invoked_Endpoint_Then_It_Should_Return_Response(attribute: "401") [8 ms]
  Error Message:
   System.Net.Http.HttpRequestException : An error occurred while sending the request.
---- System.IO.IOException : Unable to read data from the transport connection: Connection reset by peer.
-------- System.Net.Sockets.SocketException : Connection reset by peer
  Stack Trace:
     at System.Net.Http.HttpConnection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at Microsoft.Extensions.Http.Logging.LoggingHttpMessageHandler.<SendCoreAsync>g__Core|5_0(HttpRequestMessage request, Boolean useAsync, CancellationToken cancellationToken)
   at Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.<SendCoreAsync>g__Core|5_0(HttpRequestMessage request, Boolean useAsync, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.GetStringAsyncCore(HttpRequestMessage request, CancellationToken cancellationToken)
   at AzureOpenAIProxy.AppHost.Tests.ApiApp.Endpoints.GetDeploymentModelsOpenApiTests.Given_Resource_When_Invoked_Endpoint_Then_It_Should_Return_Response(String attribute) in /home/runner/work/azure-openai-sdk-proxy/azure-openai-sdk-proxy/test/AzureOpenAIProxy.AppHost.Tests/ApiApp/Endpoints/GetDeploymentModelsOpenApiTest.cs:line [14](https://github.com/aliencube/azure-openai-sdk-proxy/actions/runs/11183020416/job/31090626061?pr=281#step:10:15)1
--- End of stack trace from previous location ---
----- Inner Stack Trace -----
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.ThrowException(SocketError error, CancellationToken cancellationToken)
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.System.Threading.Tasks.Sources.IValueTaskSource<System.Int32>.GetResult(Int[16](https://github.com/aliencube/azure-openai-sdk-proxy/actions/runs/11183020416/job/31090626061?pr=281#step:10:17) token)
   at System.Net.Http.HttpConnection.InitialFillAsync(Boolean async)
   at System.Net.Http.HttpConnection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
----- Inner Stack Trace -----

Comment on lines 61 to 78
public async Task Given_Instance_When_CreateEvent_Invoked_Then_It_Should_Not_Throw_Exception()
{
// Arrange
var settings = Substitute.For<StorageAccountSettings>();
var tableServiceClient = Substitute.For<TableServiceClient>();
var tableClient = Substitute.For<TableClient>();
tableServiceClient.GetTableClient(Arg.Any<string>()).Returns(tableClient);
tableClient.AddEntityAsync<AdminEventDetails>(Arg.Any<AdminEventDetails>())
.Returns(Task.FromResult(default(Response)));
var eventDetails = new AdminEventDetails();
var repository = new AdminEventRepository(tableServiceClient, settings);

// Act
Func<Task> func = () => repository.CreateEvent(eventDetails);

// Assert
await func.Should().NotThrowAsync<InvalidOperationException>();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

아래에서 예외가 발생한다는 것을 테스트했고, 여기서는 예외가 발생하지 않고 결과를 반환한다는 것을 테스트하면 좋겠습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

repository.CreateEvent는 예외가 발생하지 않으면 메서드 인수를 그대로 반환합니다.
그래서 result.Should().BeEquivalentTo(eventDetails); 리턴값이 인수와 동일한지 확인했습니다!

Comment on lines 34 to 49
public async Task Given_Instance_When_CreateEvent_Invoked_Then_It_Should_Not_Throw_Exception()
{
// Arrange
var eventDetails = new AdminEventDetails();

var repository = Substitute.For<IAdminEventRepository>();
var service = new AdminEventService(repository);

repository.CreateEvent(Arg.Any<AdminEventDetails>()).Returns(eventDetails);

// Act
Func<Task> func = async () => await service.CreateEvent(eventDetails);
Func<Task> func = () => service.CreateEvent(eventDetails);

// Assert
func.Should().ThrowAsync<NotImplementedException>();
await func.Should().NotThrowAsync<InvalidOperationException>();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도 마찬가지로 예외를 반환하지 않고 결과값을 반환한다는 것을 테스트합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영했습니다!

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.

[Backend API] Implement endpoint for create event details
3 participants