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

Bump Misk & add audit client #427

Merged
merged 5 commits into from
Apr 4, 2025
Merged

Bump Misk & add audit client #427

merged 5 commits into from
Apr 4, 2025

Conversation

adrw
Copy link
Collaborator

@adrw adrw commented Apr 1, 2025

Misk 2025.04.01.184033-1915641

@adrw adrw requested a review from mpawliszyn April 1, 2025 19:45
eventSource = "backfila",
eventTarget = "ChickenSandwich",
timestampSent = 2147483647,
applicationName = "deep-fryer",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mpawliszyn can you double check all these values are as expected?

@adrw adrw force-pushed the 2025-04-01.auditCilent branch 3 times, most recently from 7ebb0e5 to 407f205 Compare April 2, 2025 19:41
@adrw
Copy link
Collaborator Author

adrw commented Apr 2, 2025

Waiting on help to fix Vitess changes

Copy link
Collaborator

@mpawliszyn mpawliszyn left a comment

Choose a reason for hiding this comment

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

Otherwise looks good.

@@ -77,6 +78,7 @@ dependencies {

testImplementation(libs.miskTesting)
testImplementation(libs.miskHibernateTesting)
testImplementation(testFixtures(libs.miskAuditClient))
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice!

override fun runCompleted(id: Id<DbBackfillRun>) {
val (backfillName, run, description) = transacter.transaction { session ->
val run = session.load<DbBackfillRun>(id)
Triple(run.registered_backfill.name, run, "Backfill completed ${dryRunPrefix(run)}${nameAndId(run)}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably just create a local data class?

Copy link
Collaborator

Choose a reason for hiding this comment

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

At first I thought this was the backfill description.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then we also won't be letting Db objects escape the session.

@adrw adrw force-pushed the 2025-04-01.auditCilent branch from a6edc27 to 9e4623f Compare April 4, 2025 15:46
@adrw adrw force-pushed the 2025-04-01.auditCilent branch from 5261a39 to c301320 Compare April 4, 2025 17:43

private data class AuditEvent(
val backfillName: String,
val run: DbBackfillRun,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The point is not to have the Db object?

@adrw adrw force-pushed the 2025-04-01.auditCilent branch from b1b17d7 to df18b61 Compare April 4, 2025 18:52
@mpawliszyn mpawliszyn enabled auto-merge (squash) April 4, 2025 18:56
@adrw adrw force-pushed the 2025-04-01.auditCilent branch 9 times, most recently from 1199a2f to 3963044 Compare April 4, 2025 19:22
@adrw adrw force-pushed the 2025-04-01.auditCilent branch from 3963044 to 8dbac6f Compare April 4, 2025 19:24
@mpawliszyn mpawliszyn merged commit 3f7db87 into master Apr 4, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants