Use Structured Concurrency #255

MahdiBM · 2024-10-13T08:48:39Z

Use ServiceLifecycle
Remove all occurrences of unstructured Task {}s

…ny-bot into mmbm-service-lifecycle

MahdiBM · 2024-10-21T14:36:10Z

Sources/Penny/BotStateManager.swift

+        switch Constants.deploymentEnvironment {
+        case .local:
+            break
+        case .prod:
+            self.context.backgroundProcessor.process {
+                await self.cancelIfCachePopulationTakesTooLong()
+            }
+            self.context.backgroundProcessor.process {
+                await self.send(.shutdown)
+            }
+        }


Some env checking to make sure i can manually run Penny without worrying about interrupting the prod penny.
Other than that the file changes just move stuff to structured concurrency.

MahdiBM · 2024-10-21T14:48:07Z

Sources/Penny/Services/AutoFaqsService/DefaultAutoFaqsService.swift

+    private func setUpResetItemsTask() async {
        self.resetItemsTask?.cancel()
-        self.resetItemsTask = Task {
+        let task = Task<Void, Never> {
            /// Force-refresh cache after 6 hours of no activity
            if (try? await Task.sleep(for: .seconds(60 * 60 * 6))) != nil {
                self._cachedItems = nil
-                self.getFreshItemsForCache()
-                self.setUpResetItemsTask()
+                await self.getFreshItemsForCache()
+                await self.setUpResetItemsTask()
            } else {
                /// If canceled, set up the task again.
                /// This way, the functions above can cancel this when they've got fresh items
                /// and this will just reschedule itself for a later time.
-                self.setUpResetItemsTask()
+                await self.setUpResetItemsTask()
            }
        }
+        self.resetItemsTask = task
+        await task.value
    }


this is Task, but not unstructured.

MahdiBM · 2024-10-21T14:49:21Z

Sources/Penny/WaiterService.swift

+    /// - Parameters:
+    ///   - underlyingService: The underlying service to be run after the continuation is resolved.
+    ///   - logger: A logger to log with.
+    ///   - backgroundProcessor: To process the continuation with.
+    ///   - passContinuation: Passes continuation using this closure to any other service you'd like.
+    ///   Then the other service is responsible for correctly and timely resolving the continuation.
+    init(
+        underlyingService: UnderlyingService,
+        logger: Logger = Logger(label: _typeName(Self.self)),
+        processingOn backgroundProcessor: BackgroundProcessor,
+        passingContinuationWith passContinuation: @Sendable @escaping (CheckedContinuation<Void, Never>) async -> Void
+    ) {


Some explanation.

MahdiBM · 2024-10-21T14:49:42Z

Tests/PennyTests/Fake/FakeMainService.swift

@@ -23,7 +25,7 @@ actor FakeMainService: MainService {
        cacheStorage.guilds[TestData.vaporGuild.id] = TestData.vaporGuild
        self.cache = await DiscordCache(
            gatewayManager: manager,
-            intents: [.guilds, .guildMembers],
+            intents: [.guilds, .guildMembers, .messageContent, .guildMessages],


Make it the same as the real app.

MahdiBM · 2024-10-21T14:50:27Z

Tests/PennyTests/Tests/GatewayProcessingTests.swift

+            mainServiceTask = Task<Void, any Error> {
                try await Penny.start(mainService: fakeMainService)
            }
            await fakeMainService.waitForStateManagerShutdownAndDidShutdownSignals()
        }
+
+        deinit {
+            mainServiceTask.cancel()
+        }


We cancel the task so it's not really unstructured.

MahdiBM · 2024-10-21T15:28:01Z

Sources/Penny/MainService/PennyService.swift

+        /// Services that need to wait for bot connection
+        let botStateManagerWrappedService = WaiterService(
+            underlyingService: context.botStateManager,
+            processingOn: context.backgroundProcessor,
+            passingContinuationWith: {
+                await context.discordEventListener.addConnectionWaiterContinuation($0)
+            }
+        )
+
+        /// Services that need to wait for caches population
+        let evolutionCheckerWrappedService = WaiterService(
+            underlyingService: context.evolutionChecker,
+            processingOn: context.backgroundProcessor,
+            passingContinuationWith: {
+                await context.botStateManager.addCachesPopulationContinuation($0)
+            }
+        )
+        let soCheckerWrappedService = WaiterService(
+            underlyingService: context.soChecker,
+            processingOn: context.backgroundProcessor,
+            passingContinuationWith: {
+                await context.botStateManager.addCachesPopulationContinuation($0)
+            }
+        )
+        let swiftReleasesCheckerWrappedService = WaiterService(
+            underlyingService: context.swiftReleasesChecker,
+            processingOn: context.backgroundProcessor,
+            passingContinuationWith: {
+                await context.botStateManager.addCachesPopulationContinuation($0)
+            }
+        )


See comments as to why we're using "wrapped" services.
The WaiterService type also has some doc comments.

MahdiBM · 2024-10-21T15:28:49Z

Tests/PennyTests/Tests/GatewayProcessingTests.swift

+    @Test
+    func waiterServiceRunsUnderlyingService() async throws {
+        actor SampleService: Service {
+            var didRun = false
+            func run() async throws {
+                self.didRun = true
+            }
+        }
+
+        let sampleService = SampleService()
+
+        let wrappedService = WaiterService(
+            underlyingService: sampleService,
+            processingOn: context.backgroundProcessor,
+            passingContinuationWith: { await self.context.botStateManager.addCachesPopulationContinuation($0) }
+        )
+
+        try await wrappedService.run()
+
+        #expect(await sampleService.didRun == true)
+    }
+
+    @Test
+    func waiterServiceWaitsForUnderlyingService() async throws {
+        actor SampleService: Service {
+            var didRun = false
+            func run() async throws {
+                self.didRun = true
+            }
+        }
+
+        let sampleService = SampleService()
+
+        let wrappedService = WaiterService(
+            underlyingService: sampleService,
+            processingOn: context.backgroundProcessor,
+            passingContinuationWith: { _ in /* Do nothing */ }
+        )
+
+        let runningService = Task<Void, Never> {
+            try! await wrappedService.run()
+        }
+
+        try await Task.sleep(for: .seconds(5))
+
+        runningService.cancel()
+
+        #expect(await sampleService.didRun == false)
+    }


Some tests for the waiter service

MahdiBM · 2024-10-21T15:29:49Z

Tests/PennyTests/Tests/GatewayProcessingTests.swift

+        let serviceTask = Task<Void, any Error> { [self] in
+            try await self.context.swiftReleasesChecker.run()
+        }


This Task isn't really "unstructured" since we cancel it in the end.

Add ServiceLifecycle

ef4a24d

MahdiBM changed the base branch from main to mmbm-swift-releases-checker October 13, 2024 08:48

MahdiBM mentioned this pull request Oct 13, 2024

SwiftReleasesChecker to report new Swift releases #254

Merged

MahdiBM marked this pull request as ready for review October 13, 2024 12:10

MahdiBM requested review from 0xTim and gwynne as code owners October 13, 2024 12:10

MahdiBM marked this pull request as draft October 13, 2024 13:58

Base automatically changed from mmbm-swift-releases-checker to main October 13, 2024 18:22

MahdiBM added 11 commits October 13, 2024 22:23

Swift Releases Checker

ded5046

better test

7a26b6f

increase BotStateManager wait time

c120e2d

Update SwiftReleasesChecker.swift

9905929

Actually run the service too

2f6eab8

get most of it working

f8ec5e0

move almost everything to structured concurrency

2f4ca20

more cleanup

3502459

build

f5f0ffc

revert some unneeded change

23cb8af

Update Errors.swift

bfee243

MahdiBM force-pushed the mmbm-service-lifecycle branch from 1f4e208 to bfee243 Compare October 13, 2024 18:53

MahdiBM added 7 commits October 13, 2024 22:26

Merge branch 'main' into mmbm-service-lifecycle

a1c0be2

revert

924ed70

Merge branch 'mmbm-service-lifecycle' of https://github.com/vapor/pen…

b0295bc

…ny-bot into mmbm-service-lifecycle

remove unneeded lock

1c25ac1

fix tests

9779b8b

add envs

3e7b097

minor fixes

438682b

MahdiBM marked this pull request as ready for review October 20, 2024 08:50

MahdiBM added 2 commits October 20, 2024 12:23

remove some imports

9ad5430

Update Constants.swift

776ac94

MahdiBM added 7 commits October 21, 2024 16:22

Update test.yml

13a8841

Update PennyService.swift

0573deb

fix tests build

c8b21ec

Update GHHooksTests.swift

c292f43

connect the bot in DiscordListener

aa774de

Merge branch 'main' into mmbm-service-lifecycle

eae9123

remove an import

e591f96

MahdiBM commented Oct 21, 2024

View reviewed changes

MahdiBM added 2 commits October 21, 2024 18:12

minor refinements

f8385c4

doc comments

9086f5f

MahdiBM commented Oct 21, 2024

View reviewed changes

botStateManager wait for bot ws connection

e6e49e8

MahdiBM commented Oct 21, 2024

View reviewed changes

MahdiBM added 12 commits October 21, 2024 20:45

fix calling data-refresh logic

34461b7

refinements for local running

d9dda8e

Update EvolutionChecker.swift

a485b13

limit concurrent tasks in the BackgroundProcessor to 8

b20aceb

better gateway readiness

4925237

Update DiscordEventListener.swift

edaf448

try macOS CI fix

d92a94e

revert back to main vapor CI repo

9149b6e

Merge branch 'main' into mmbm-service-lifecycle

a68ac53

Merge branch 'main' into mmbm-service-lifecycle

7f68522

Merge branch 'main' into mmbm-service-lifecycle

f6a1515

Merge branch 'main' into mmbm-service-lifecycle

e70db54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Structured Concurrency #255

Use Structured Concurrency #255

MahdiBM commented Oct 13, 2024 •

edited

Loading

MahdiBM Oct 21, 2024

MahdiBM Oct 21, 2024 •

edited

Loading

MahdiBM Oct 21, 2024

MahdiBM Oct 21, 2024

MahdiBM Oct 21, 2024

MahdiBM Oct 21, 2024

MahdiBM Oct 21, 2024

MahdiBM Oct 21, 2024

Use Structured Concurrency #255

Are you sure you want to change the base?

Use Structured Concurrency #255

Conversation

MahdiBM commented Oct 13, 2024 • edited Loading

MahdiBM Oct 21, 2024

Choose a reason for hiding this comment

MahdiBM Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

MahdiBM Oct 21, 2024

Choose a reason for hiding this comment

MahdiBM Oct 21, 2024

Choose a reason for hiding this comment

MahdiBM Oct 21, 2024

Choose a reason for hiding this comment

MahdiBM Oct 21, 2024

Choose a reason for hiding this comment

MahdiBM Oct 21, 2024

Choose a reason for hiding this comment

MahdiBM Oct 21, 2024

Choose a reason for hiding this comment

MahdiBM commented Oct 13, 2024 •

edited

Loading

MahdiBM Oct 21, 2024 •

edited

Loading