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

fix(recordings): fix bug caused by #1692 #1746

Merged
merged 2 commits into from
Oct 30, 2023

Conversation

mwangggg
Copy link
Member

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Fixes: #1738
Related: #1731 #1692

@github-actions
Copy link
Contributor

Hi @mwangggg! Add at least one of the required labels to this PR

Required labels are : chore,ci,cleanup,docs,feat,fix,perf,refactor,style,test

@mergify mergify bot added the safe-to-test label Oct 25, 2023
@github-actions
Copy link
Contributor

Hi @mwangggg! Add at least one of the required labels to this PR

Required labels are : chore,ci,cleanup,docs,feat,fix,perf,refactor,style,test

@andrewazores andrewazores changed the title fix(recordings): fix bug caused by #1731 fix(recordings): fix bug caused by #1692 Oct 25, 2023
@mwangggg
Copy link
Member Author

/build_test

@mwangggg mwangggg marked this pull request as ready for review October 27, 2023 19:47
@andrewazores
Copy link
Member

Nice, very small fix after all. I tested this alongside cryostatio/cryostat-web#1127 and I now see that the web-client makes the requests to the server using the unencoded JVM ID, and then the server internally does the encoding/decoding to deal with the filesystem and completes the request.

@andrewazores
Copy link
Member

Looks like this needs a spotless:apply though.

@mwangggg mwangggg requested review from andrewazores, aali309 and a team October 30, 2023 13:49
@andrewazores andrewazores merged commit d80f622 into cryostatio:main Oct 30, 2023
7 checks passed
mergify bot pushed a commit that referenced this pull request Oct 30, 2023
andrewazores pushed a commit that referenced this pull request Oct 30, 2023
(cherry picked from commit d80f622)

Co-authored-by: Ming Yu Wang <90855268+mwangggg@users.noreply.github.com>
@mwangggg mwangggg deleted the fix-jvmId-bug branch March 12, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] Archives > All Archives > Download does not download JFR file
2 participants