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

Update name of remote recording before allowing the AppMap indexer to index it #479

Merged
merged 3 commits into from
Oct 24, 2023

Conversation

jansorg
Copy link
Collaborator

@jansorg jansorg commented Oct 18, 2023

Closes #477

This streams the AppMap JSON of the remote recording to a temporary file, then updates the metadata.name property of this file and then moves it to the final file, which has a .appmap.json extension.

This PR re-enables test appland.remote.DefaultRemoteRecordingServiceTest to cover the remote recording implementation. Previously, it was disabled due to incompatibility with an active AppMap agent.

@github-actions
Copy link

github-actions bot commented Oct 18, 2023

AppMap pull request analysis

Summary Status
Failed tests ✅ All tests passed
API changes 0️⃣ No API changes
Security flaws ✅ None detected
Performance problems ✅ None detected
Code anti-patterns ✅ None detected
New AppMaps 0️⃣ No new AppMaps

@jansorg jansorg force-pushed the jansorg/appmap-naming branch from 307b724 to 182e840 Compare October 18, 2023 12:37
@jansorg jansorg requested a review from ahtrotta October 18, 2023 12:40
ahtrotta
ahtrotta previously approved these changes Oct 19, 2023
Copy link
Contributor

@ahtrotta ahtrotta left a comment

Choose a reason for hiding this comment

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

Looks good and worked well when I tested it, thanks!

@ahtrotta ahtrotta dismissed their stale review October 19, 2023 16:50

Build failing

@ahtrotta ahtrotta self-requested a review October 19, 2023 16:51
@ahtrotta
Copy link
Contributor

Can you look into why the Travis build is failing?

@jansorg
Copy link
Collaborator Author

jansorg commented Oct 19, 2023

@jansorg jansorg force-pushed the jansorg/appmap-naming branch from 182e840 to 435414f Compare October 24, 2023 09:31
Copy link
Contributor

@ahtrotta ahtrotta left a comment

Choose a reason for hiding this comment

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

Thanks @jansorg!

@ahtrotta ahtrotta merged commit 1bcf1e0 into develop Oct 24, 2023
@ahtrotta ahtrotta deleted the jansorg/appmap-naming branch October 24, 2023 15:18
@appland-release
Copy link
Contributor

🎉 This PR is included in version 0.52.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent naming for remote recording appmaps
3 participants