-
Notifications
You must be signed in to change notification settings - Fork 109
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
fix: not ingesting tenant count during azure analysis #328
Conversation
KeyVaults: 1, | ||
Relationships: 188, | ||
} | ||
_, agg, err := azure2.GraphStats(context.TODO(), testCtx.GraphDB) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opted to skip verifying the individual stats slice of structs here because building the expected result involves retrieving a runId which is generated inside of graphStats fn itself, as well as all the hardcode above x12. If anyone changes the harnesses, the agg will fail this test anyway. That said, I'm open to any solutions that might get us a less brittle test here.
@@ -48,7 +48,7 @@ func GraphStats(ctx context.Context, db graph.Database) (model.AzureDataQualityS | |||
runID = newUUID.String() | |||
} | |||
|
|||
return stats, aggregation, db.ReadTransaction(ctx, func(tx graph.Transaction) error { | |||
err := db.ReadTransaction(ctx, func(tx graph.Transaction) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is just for readability.
b3159ac
to
b6789d2
Compare
@@ -1042,6 +1068,7 @@ type AZInboundControlHarness struct { | |||
|
|||
func (s *AZInboundControlHarness) Setup(testCtx *GraphTestContext) { | |||
tenantID := RandomObjectID(testCtx.testCtx) | |||
s.Tenant = testCtx.NewAzureTenant(tenantID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON/SVGs will need to be updated here like we did for the other one above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me apart from the changes we discussed (added in comments). Go ahead and merge once those updates are made!
Description
When selecting “All Azure Tenants” in Data Quality, the statistics table shows 0 (zero) tenants.
Motivation and Context
All Azure Tenants stats should accurately display tenant count.
How Has This Been Tested?
Integration test has been added.
Screenshots (if appropriate):
Types of changes
Checklist: