Skip to content

Conversation

@aaronburtle
Copy link
Contributor

@aaronburtle aaronburtle commented Dec 7, 2022

Why make this change?

Closes #968

We are currently logging only those entities loaded for REST, this adds GQL entities to the logging.

What is this change?

On startup when we map the GQL types to entity names we log the relevant entities.

This happens within MapGraphQLSingularTypeToEntityName

How was this tested?

Verified on startup that the calls are made into the logger when appropriate.

Sample Request(s)

Startup the service with valid entities for GQL.

Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Need to fix what we log as GraphQL Entity

@Aniruddh25
Copy link
Collaborator

nit: the description should provide the exact issue number after the phrase Closes. Instead of a link to all the issues.

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

Might be worth considering whether delaying the logging of graphQL entity names

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

last few suggestions

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

Thanks for addressing comments. looks concise. Two small change suggestions and should be good to go.

Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
@aaronburtle aaronburtle merged commit a1d77dc into main Dec 8, 2022
@aaronburtle aaronburtle deleted the dev/aaronburtle/LogGQLEntityInformation branch December 8, 2022 07:39
@aaronburtle aaronburtle added this to the Nov2022 milestone Dec 8, 2022
@aaronburtle aaronburtle added the improvement Let's make this better label Dec 8, 2022
@aaronburtle aaronburtle self-assigned this Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Let's make this better

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Log entity information

5 participants