Skip to content

refactor(keycloak): migrate Keycloak plugin to NestJS#1995

Open
shikanime wants to merge 1 commit intomainfrom
pr1995
Open

refactor(keycloak): migrate Keycloak plugin to NestJS#1995
shikanime wants to merge 1 commit intomainfrom
pr1995

Conversation

@shikanime
Copy link
Contributor

@shikanime shikanime commented Mar 12, 2026

@shikanime
Copy link
Contributor Author

@StephaneTrebel Pas besoin d'une vrai review, je voudrais avoir ton avis sur la direction et voir ce que je devrais faire differement

@shikanime shikanime force-pushed the pr1995 branch 10 times, most recently from b69f23c to 51696fe Compare March 12, 2026 16:49
@shikanime shikanime force-pushed the pr1995 branch 8 times, most recently from 72bd97c to 7340cbf Compare March 13, 2026 09:05

@OnEvent('project.upsert')
async handleUpsert(project: ProjectWithDetails) {
return tracer.startActiveSpan('handleUpsert', async (span) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion(blocker): Oulà c'est bien lourd, tout ça, et ça obfusque la vraie signature de la fonction. On serait mieux avisé d'avoir un décorateur pour ça, non ?

https://oneuptime.com/blog/post/2026-02-06-custom-opentelemetry-spans-nestjs-controllers-services/view#creating-a-span-decorator

@StephaneTrebel
Copy link
Collaborator

@StephaneTrebel Pas besoin d'une vrai review, je voudrais avoir ton avis sur la direction et voir ce que je devrais faire differement

C'est pas mal du tout 👍

  • Faut qu'on revoie l'histoire du tracing pour que son abstraction soit la plus légère possible
  • Ptet prévoir davantage de tests unitaires pour les cas tordus: on a beaucoup de complexité cyclomatique qui se balade un peu partout. Ma crainte c'est d'arriver à ce qu'on "ne sorte pas des rails", surtout sur ce sujet sensible
  • Manque peut-être un peu de doc qui explique les tenants et les aboutissants entres les services, le flux des données, les cas d'usages, etc.

)
}
await prisma.$connect()
await this.prisma.$connect()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should let the prisma service handle it

@shikanime shikanime force-pushed the pr1995 branch 3 times, most recently from 50df5fe to 8e29c2f Compare March 13, 2026 16:20
Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
@cloud-pi-native-sonarqube
Copy link

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

Labels

tech Technical issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants