Gitea workflow trigger #18
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -110,13 +110,13 @@ importers: | |||
| version: 6.0.2 | |||
| eslint: | |||
| specifier: ^9.18.0 | |||
| version: 9.21.0 | |||
| version: 9.21.0([email protected]) | |||
There was a problem hiding this comment.
The version format 9.21.0([email protected]) seems incorrect. Typically, the version should be a simple semantic version number. Please verify if this is the intended format or if it should be corrected.
| eslint-config-prettier: | ||
| specifier: ^10.0.1 | ||
| version: 10.0.1([email protected]) | ||
| version: 10.0.1([email protected]([email protected])) |
There was a problem hiding this comment.
The version format 10.0.1([email protected]([email protected])) appears unusual. Ensure that this is the correct format for specifying dependencies in pnpm-lock.yaml.
| eslint-plugin-prettier: | ||
| specifier: ^5.2.2 | ||
| version: 5.2.3(@types/[email protected])([email protected]([email protected]))([email protected])([email protected]) | ||
| version: 5.2.3(@types/[email protected])([email protected]([email protected]([email protected])))([email protected]([email protected]))([email protected]) |
There was a problem hiding this comment.
The version string 5.2.3(@types/[email protected])([email protected]([email protected]([email protected])))([email protected]([email protected]))([email protected]) is complex and may not be correctly formatted. Please confirm that this is the intended structure for the version and dependencies.
| @@ -106,6 +107,9 @@ export class SubmissionService { | |||
| } | |||
|
|
|||
| async getSubmission(submissionId: string): Promise<SubmissionResponseDto> { | |||
| // TODO mocked data only, remove when challenge is concluded | |||
There was a problem hiding this comment.
The use of mocked data should be clearly tracked to ensure it is removed after the challenge is concluded. Consider implementing a mechanism to switch between mocked and real data based on an environment variable or configuration setting.
There was a problem hiding this comment.
@billsedison this method should be using live data not mocked data. Please update.
src/mock/mock.data.ts
Outdated
| legacyId: 54321, | ||
| workflows: [ | ||
| { | ||
| worflowId: 'test_ai_workflow.yaml', |
There was a problem hiding this comment.
The property worflowId seems to be misspelled. Consider changing it to workflowId for consistency and accuracy.
src/mock/mock.data.ts
Outdated
| params: { key: 'value' }, | ||
| }, | ||
| { | ||
| worflowId: 'test_ai_workflows.yaml', |
There was a problem hiding this comment.
The property worflowId seems to be misspelled. Consider changing it to workflowId for consistency and accuracy.
src/mock/mock.data.ts
Outdated
| legacyId: 123, | ||
| workflows: [ | ||
| { | ||
| worflowId: 'test_ai_workflow.yaml', |
There was a problem hiding this comment.
The property worflowId seems to be misspelled. Consider changing it to workflowId for consistency and accuracy.
src/mock/mock.data.ts
Outdated
| params: { key: 'value' }, | ||
| }, | ||
| { | ||
| worflowId: 'test_ai_workflows.yaml', |
There was a problem hiding this comment.
The property worflowId seems to be misspelled. Consider changing it to workflowId for consistency and accuracy.
src/mock/mock.data.ts
Outdated
| id: '67890', | ||
| challengeId: 'challenge-id-123', | ||
| type: 'code', | ||
| url: 'https://example.com/submission/12345', |
There was a problem hiding this comment.
The URL for the submission with id '67890' is the same as the one for '12345'. Consider updating it to ensure it points to the correct resource.
| @@ -6,6 +6,7 @@ import { AxiosError } from 'axios'; | |||
| import { M2MService } from './m2m.service'; | |||
| import { Injectable, Logger } from '@nestjs/common'; | |||
| import { CommonConfig } from 'src/shared/config/common.config'; | |||
| import { MockedData } from 'src/mock/mock.data'; | |||
There was a problem hiding this comment.
The import statement for MockedData suggests that mock data is being used in a service file. Ensure that this is intended for production code, as mock data should typically be used in testing environments. If this is for testing purposes, consider moving it to a test file or environment-specific configuration.
| } | ||
|
|
||
| export class WorkflowData { | ||
| worflowId: string; |
There was a problem hiding this comment.
Typo in property name: worflowId should be workflowId.
| @@ -38,6 +46,9 @@ export class ChallengeApiService { | |||
| } | |||
|
|
|||
| async getChallengeDetail(challengeId: string): Promise<ChallengeData> { | |||
| // TODO mocked data only, remove when challenge is concluded | |||
There was a problem hiding this comment.
The use of mocked data should be clearly tracked to ensure it is removed before production. Consider implementing a mechanism to switch between mocked and real data based on the environment or configuration.
| challengeId: string, | ||
| ): Promise<void> { | ||
| this.logger.log( | ||
| `Running workflow: ${workflow.worflowId} with ref: ${workflow.ref}`, |
There was a problem hiding this comment.
Typo in the log message: worflowId should be workflowId. Ensure consistency in naming.
| this.logger.log(`Workflow dispatched successfully: ${response.status}`); | ||
| } catch (error) { | ||
| this.logger.error( | ||
| `Error dispatching workflow ${workflow.worflowId}: ${error.message}`, |
There was a problem hiding this comment.
Typo in the error message: worflowId should be workflowId. Correct the spelling to match the parameter name.
| try { | ||
| await this.giteaService.runDispatchWorkflow(workflow, challenge.id); | ||
| } catch (error) { | ||
| const errorMessage = `Error processing workflow: ${workflow.worflowId}. Error: ${error.message}.`; |
There was a problem hiding this comment.
Typo in the error message: worflowId should be workflowId.
| constructor(private readonly handlerRegistry: KafkaHandlerRegistry) { | ||
| constructor( | ||
| private readonly handlerRegistry: KafkaHandlerRegistry, | ||
| private readonly orchestrator: SubmissionScanCompleteOrchestrator, |
There was a problem hiding this comment.
The new dependency SubmissionScanCompleteOrchestrator is added to the constructor, but it is not clear how it is being used in this class. Ensure that this dependency is necessary and utilized within the class methods.
| @@ -37,11 +41,18 @@ export class SubmissionScanCompleteHandler | |||
| } | |||
|
|
|||
| this.logger.log('=== Submission Scan Complete Event ==='); | |||
| this.logger.log('Topic:', this.topic); | |||
| this.logger.log('Payload:', JSON.stringify(message, null, 2)); | |||
| this.logger.log('Topic: ' + this.topic); | |||
There was a problem hiding this comment.
The change from using comma-separated arguments to string concatenation for logging might affect readability and consistency. Consider using template literals for better readability: this.logger.log(Topic: ${this.topic});
| this.logger.log('Topic:', this.topic); | ||
| this.logger.log('Payload:', JSON.stringify(message, null, 2)); | ||
| this.logger.log('Topic: ' + this.topic); | ||
| this.logger.log('Payload: ' + JSON.stringify(message, null, 2)); |
There was a problem hiding this comment.
Similar to the previous comment, consider using template literals for logging the payload for consistency and readability: this.logger.log(Payload: ${JSON.stringify(message, null, 2)});
| @@ -106,6 +107,9 @@ export class SubmissionService { | |||
| } | |||
|
|
|||
| async getSubmission(submissionId: string): Promise<SubmissionResponseDto> { | |||
| // TODO mocked data only, remove when challenge is concluded | |||
There was a problem hiding this comment.
@billsedison this method should be using live data not mocked data. Please update.
| } | ||
|
|
||
| export class WorkflowData { | ||
| workflowId: string; |
There was a problem hiding this comment.
Typo correction: The property name worflowId has been corrected to workflowId. Ensure that all references to this property in the codebase are updated accordingly to prevent any runtime errors or undefined behavior.
| challengeId: string, | ||
| ): Promise<void> { | ||
| this.logger.log( | ||
| `Running workflow: ${workflow.workflowId} with ref: ${workflow.ref}`, |
There was a problem hiding this comment.
Typo correction: Ensure that the variable workflow.workflowId is correctly defined and used throughout the codebase, as it was previously worflowId. Double-check for consistency in naming.
| const response = await this.giteaClient.repos.actionsDispatchWorkflow( | ||
| this.giteaOrg, | ||
| challengeId, | ||
| workflow.workflowId, |
There was a problem hiding this comment.
Typo correction: Verify that the change from worflowId to workflowId is consistent across all related files and usages to prevent any potential errors.
| try { | ||
| await this.giteaService.runDispatchWorkflow(workflow, challenge.id); | ||
| } catch (error) { | ||
| const errorMessage = `Error processing workflow: ${workflow.workflowId}. Error: ${error.message}.`; |
There was a problem hiding this comment.
There is a typo in the original code that has been corrected in this diff. Ensure that the variable workflow.workflowId is correctly defined and used throughout the codebase, as the original code had worflowId which seems to be a typo.
No description provided.