[DevTools] PR3.5 Deployment Tracking #2917
Conversation
🦋 Changeset detectedLatest commit: 533e161 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Didn't go super deep on this yet; but ran into a newly introduced class I didn't actually see being imported anywhere. Is it supposed to be part of a different PR?
Ideally, let's make sure this PR only includes its own relevant changes -- or, if it's even possible, a convincing note indicating why a change is included here but not yet consumed.
| const eventMap = new Map(); | ||
|
|
||
| // Add existing events to the map | ||
| existingEvents.forEach((event) => { | ||
| const key = | ||
| event.resourceStatus?.eventId || | ||
| //eslint-disable-next-line @typescript-eslint/restrict-template-expressions | ||
| `${event.timestamp}-${event.message}`; | ||
| eventMap.set(key, event); | ||
| }); | ||
|
|
||
| // Add new events to the map (will overwrite duplicates) | ||
| formattedEvents.forEach((event) => { | ||
| const key = | ||
| event.resourceStatus?.eventId || | ||
| `${event.timestamp}-${event.message}`; | ||
| eventMap.set(key, event); | ||
| }); | ||
|
|
||
| // Convert map back to array | ||
| const mergedEvents = Array.from(eventMap.values()); |
There was a problem hiding this comment.
Same as mergeEvents() method here. Might be good as a shared util.
| cfnEventsInterval = setInterval(() => { | ||
| console.log(`Polling for CloudFormation events, status: ${status}`); | ||
| deploymentClientService.getCloudFormationEvents(); | ||
| }, 5000); |
There was a problem hiding this comment.
Maybe better as a constant with note indicating why 5000.
| console.log( | ||
| `Setting up polling for CloudFormation events (${status} state)`, | ||
| ); | ||
| cfnEventsInterval = setInterval(() => { |
There was a problem hiding this comment.
Having a hard time tracing through some of this; but are we polling from the local service for things that could be better pushed to the front-end via the socket?
Not a deal-breaker if we are; but if my sense is right on that, I'd at least appreciate a comment indicating why.
| /** | ||
| * Service for handling socket events related to resources | ||
| */ | ||
| export class SocketHandlerResources { |
megha-narayanan
force-pushed
the
pr-3.5-deployment
branch
from
cf0f1bc to
533e161
Compare
2 participants
End-to-end deployment tracking.
Note there had been an intention earlier to have Deployment Progress share more utilities with cfn_progress_logger, though partially due to time constraints, I have opted to stick with my current implementation that also provides critical persistence which is not possible by simply using cfn_progress_logger. Part of the purpose of devtools is being able to debug, which means the ability to go and see past deployment events is important.