Skip to content

Commit 08464da

Browse files
leogdionclaude
andcommitted
refactor: address PR review comments
- Clean up redundant envPrefix comments in ConfigurationKeys.swift - Added section-level docs explaining pattern once - Removed repetitive inline comments - Applied to all config enums for consistency - Refactor pemString parameter to use Optional<String> - Changed SyncEngine.init() from String = "" to String? = nil - Updated logic from isEmpty check to if-let binding - Simplified SyncCommand call site (removed ?? "") - More idiomatic Swift, better expresses optional nature - Update cron schedule to 3x daily with randomized times - Changed from every 12 hours to specific times: 02:17, 10:43, 18:29 UTC (~8 hours apart) - Randomized minutes avoid predictable patterns - Aligns with VirtualBuddy TSS 12-hour cache - Restructure workflow into separate build and sync jobs - Build job: Runs in Swift container, compiles binary - Sync job: Runs on native Ubuntu (no container overhead) - Only build needs Swift container (reduced CI minutes) - Better separation of concerns Addresses review comments from @leogdion on PR #10 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent b81d6be commit 08464da

File tree

4 files changed

+61
-26
lines changed

4 files changed

+61
-26
lines changed

.github/workflows/cloudkit-sync.yml

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
name: Scheduled CloudKit Sync
22

33
on:
4-
# Run every 12 hours (00:00 and 12:00 UTC)
4+
# Run three times daily (every ~8 hours) with randomized minutes
5+
# Staggered to avoid predictable patterns and align with VirtualBuddy TSS cache (12h)
56
schedule:
6-
- cron: '0 */12 * * *'
7+
- cron: '17 2 * * *' # 02:17 UTC
8+
- cron: '43 10 * * *' # 10:43 UTC
9+
- cron: '29 18 * * *' # 18:29 UTC
710

811
# Allow manual trigger for testing
912
workflow_dispatch:
@@ -19,11 +22,11 @@ concurrency:
1922
cancel-in-progress: true
2023

2124
jobs:
22-
sync:
23-
name: Sync to CloudKit
25+
build:
26+
name: Build bushel-cloud
2427
runs-on: ubuntu-latest
2528
container: swift:6.2-noble
26-
timeout-minutes: 30
29+
timeout-minutes: 20
2730

2831
permissions:
2932
contents: read # Read repository code
@@ -54,13 +57,32 @@ jobs:
5457
echo "Binary location: $BIN_PATH/bushel-cloud"
5558
ls -lh "$BIN_PATH/bushel-cloud"
5659
57-
- name: Upload binary as artifact
60+
- name: Upload binary for sync job
5861
uses: actions/upload-artifact@v4
5962
with:
60-
name: bushel-cloud-${{ runner.os }}-${{ github.sha }}
63+
name: bushel-cloud-binary
6164
path: ${{ steps.build.outputs.bin-path }}/bushel-cloud
62-
retention-days: 7
63-
if-no-files-found: warn
65+
retention-days: 1 # Short retention, only needed for sync job
66+
if-no-files-found: error
67+
68+
sync:
69+
name: Sync to CloudKit
70+
runs-on: ubuntu-latest # No container needed - uses pre-built binary
71+
needs: [build]
72+
timeout-minutes: 30
73+
74+
permissions:
75+
contents: read # Read repository code
76+
77+
steps:
78+
- name: Download built binary
79+
uses: actions/download-artifact@v4
80+
with:
81+
name: bushel-cloud-binary
82+
path: ./binary
83+
84+
- name: Make binary executable
85+
run: chmod +x ./binary/bushel-cloud
6486

6587
- name: Run CloudKit sync
6688
env:
@@ -73,13 +95,9 @@ jobs:
7395
echo "Container: $CLOUDKIT_CONTAINER_ID"
7496
echo "Environment: $CLOUDKIT_ENVIRONMENT"
7597
76-
# Use pre-built binary (already computed in build step)
77-
BIN_PATH="${{ steps.build.outputs.bin-path }}"
78-
echo "Using binary: $BIN_PATH/bushel-cloud"
79-
8098
# Run pre-built binary
8199
# Note: Same key works for both development and production
82100
# Change CLOUDKIT_ENVIRONMENT to 'production' when ready
83-
"$BIN_PATH/bushel-cloud" sync \
101+
./binary/bushel-cloud sync \
84102
--verbose \
85103
--container-identifier "$CLOUDKIT_CONTAINER_ID"

Sources/BushelCloudCLI/Commands/SyncCommand.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ enum SyncCommand {
5353
containerIdentifier: config.cloudKit.containerID,
5454
keyID: config.cloudKit.keyID,
5555
privateKeyPath: config.cloudKit.privateKeyPath,
56-
pemString: config.cloudKit.privateKey ?? "", // Raw PEM if provided
56+
pemString: config.cloudKit.privateKey, // Raw PEM if provided (optional)
5757
environment: config.cloudKit.environment, // Parsed environment enum
5858
configuration: fetchConfiguration
5959
)

Sources/BushelCloudKit/CloudKit/SyncEngine.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,21 +78,21 @@ public struct SyncEngine: Sendable {
7878
/// - containerIdentifier: CloudKit container ID
7979
/// - keyID: Server-to-Server Key ID
8080
/// - privateKeyPath: Path to .pem file (use if pemString not provided)
81-
/// - pemString: PEM content as string (use for CI/CD, takes priority over privateKeyPath)
81+
/// - pemString: PEM content as string (use for CI/CD, takes priority over privateKeyPath when non-nil)
8282
/// - environment: CloudKit environment (.development or .production, defaults to .development)
8383
/// - configuration: Fetch configuration for data sources
8484
/// - Throws: Error if authentication credentials are invalid or missing
8585
public init(
8686
containerIdentifier: String,
8787
keyID: String,
8888
privateKeyPath: String = "",
89-
pemString: String = "",
89+
pemString: String? = nil,
9090
environment: Environment = .development,
9191
configuration: FetchConfiguration = FetchConfiguration.loadFromEnvironment()
9292
) throws {
9393
// Prefer PEM string if provided (CI/CD pattern), fall back to file path (local development)
9494
let service: BushelCloudKitService
95-
if !pemString.isEmpty {
95+
if let pemString = pemString {
9696
service = try BushelCloudKitService(
9797
containerIdentifier: containerIdentifier,
9898
keyID: keyID,

Sources/BushelCloudKit/Configuration/ConfigurationKeys.swift

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,13 @@ internal enum ConfigurationKeys {
3535
// MARK: - CloudKit Configuration
3636

3737
/// CloudKit configuration keys
38+
///
39+
/// Uses `envPrefix: nil` to auto-generate environment variable names from the key path.
40+
/// Example: "cloudkit.container_id" → ENV: CLOUDKIT_CONTAINER_ID
3841
internal enum CloudKit {
39-
// Using base key with auto-generation (no prefix for CloudKit ENV vars)
4042
internal static let containerID = ConfigKey<String>(
4143
"cloudkit.container_id",
42-
envPrefix: nil, // Generates: CLI="cloudkit.container_id", ENV="CLOUDKIT_CONTAINER_ID"
44+
envPrefix: nil,
4345
default: "iCloud.com.brightdigit.Bushel"
4446
)
4547

@@ -55,32 +57,37 @@ internal enum ConfigurationKeys {
5557

5658
internal static let privateKey = OptionalConfigKey<String>(
5759
"cloudkit.private_key",
58-
envPrefix: nil // Generates: ENV="CLOUDKIT_PRIVATE_KEY"
60+
envPrefix: nil
5961
)
6062

6163
internal static let environment = ConfigKey<String>(
6264
"cloudkit.environment",
63-
envPrefix: nil, // Generates: ENV="CLOUDKIT_ENVIRONMENT"
65+
envPrefix: nil,
6466
default: "development"
6567
)
6668
}
6769

6870
// MARK: - VirtualBuddy Configuration
6971

7072
/// VirtualBuddy TSS API configuration keys
73+
///
74+
/// Uses `envPrefix: nil` for auto-generated ENV names (VIRTUALBUDDY_API_KEY).
7175
internal enum VirtualBuddy {
7276
internal static let apiKey = OptionalConfigKey<String>(
7377
"virtualbuddy.api_key",
74-
envPrefix: nil // Generates: ENV="VIRTUALBUDDY_API_KEY"
78+
envPrefix: nil
7579
)
7680
}
7781

7882
// MARK: - Fetch Configuration
7983

8084
/// Fetch throttling configuration keys
85+
///
86+
/// Uses `bushelPrefixed:` to add BUSHEL_ prefix to all environment variables.
87+
/// Example: "fetch.interval_global" → ENV: BUSHEL_FETCH_INTERVAL_GLOBAL
8188
internal enum Fetch {
8289
internal static let intervalGlobal = OptionalConfigKey<Double>(
83-
bushelPrefixed: "fetch.interval_global" // Generates: ENV="BUSHEL_FETCH_INTERVAL_GLOBAL"
90+
bushelPrefixed: "fetch.interval_global"
8491
)
8592

8693
/// Generate per-source interval key dynamically
@@ -90,14 +97,16 @@ internal enum ConfigurationKeys {
9097
let normalized = source.replacingOccurrences(of: ".", with: "_")
9198
return OptionalConfigKey<Double>(
9299
"fetch.interval.\(normalized)",
93-
envPrefix: nil // CLI: "fetch.interval.appledb_dev", ENV: "FETCH_INTERVAL_APPLEDB_DEV"
100+
envPrefix: nil
94101
)
95102
}
96103
}
97104

98105
// MARK: - Sync Command Configuration
99106

100-
/// Sync command configuration keys (using base key with BUSHEL prefix)
107+
/// Sync command configuration keys
108+
///
109+
/// Uses `bushelPrefixed:` for BUSHEL_SYNC_* environment variables.
101110
internal enum Sync {
102111
internal static let dryRun = ConfigKey<Bool>(bushelPrefixed: "sync.dry_run")
103112
internal static let restoreImagesOnly = ConfigKey<Bool>(
@@ -115,6 +124,8 @@ internal enum ConfigurationKeys {
115124
// MARK: - Export Command Configuration
116125

117126
/// Export command configuration keys
127+
///
128+
/// Uses `bushelPrefixed:` for BUSHEL_EXPORT_* environment variables.
118129
internal enum Export {
119130
internal static let output = OptionalConfigKey<String>(bushelPrefixed: "export.output")
120131
internal static let pretty = ConfigKey<Bool>(bushelPrefixed: "export.pretty")
@@ -126,6 +137,8 @@ internal enum ConfigurationKeys {
126137
// MARK: - Status Command Configuration
127138

128139
/// Status command configuration keys
140+
///
141+
/// Uses `bushelPrefixed:` for BUSHEL_STATUS_* environment variables.
129142
internal enum Status {
130143
internal static let errorsOnly = ConfigKey<Bool>(bushelPrefixed: "status.errors_only")
131144
internal static let detailed = ConfigKey<Bool>(bushelPrefixed: "status.detailed")
@@ -134,6 +147,8 @@ internal enum ConfigurationKeys {
134147
// MARK: - List Command Configuration
135148

136149
/// List command configuration keys
150+
///
151+
/// Uses `bushelPrefixed:` for BUSHEL_LIST_* environment variables.
137152
internal enum List {
138153
internal static let restoreImages = ConfigKey<Bool>(bushelPrefixed: "list.restore_images")
139154
internal static let xcodeVersions = ConfigKey<Bool>(bushelPrefixed: "list.xcode_versions")
@@ -143,6 +158,8 @@ internal enum ConfigurationKeys {
143158
// MARK: - Clear Command Configuration
144159

145160
/// Clear command configuration keys
161+
///
162+
/// Uses `bushelPrefixed:` for BUSHEL_CLEAR_* environment variables.
146163
internal enum Clear {
147164
internal static let yes = ConfigKey<Bool>(bushelPrefixed: "clear.yes")
148165
internal static let verbose = ConfigKey<Bool>(bushelPrefixed: "clear.verbose")

0 commit comments

Comments
 (0)