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(graphql): use array format as label input #1411

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented Mar 14, 2023

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 the last commit: git commit --amend --signoff

Fixes: #1407

Description of the change:

Use array format for label input similar to metadata mutator.

Motivation for the change:

See #1407

How to test

Try this query:

query StartRecordingForGroup() {
  environmentNodes(filter: { name: "JDP" }) {
    descendantTargets {
      doStartRecording(recording: {
        name: "foo",
        template: "Continuous",
        templateType: "TARGET",
        duration: 0,
        restart: true,
        metadata: {
          labels: [
            {
              key: "cryostat.io.topology-group",
              value: "JDP"
            }
          ]
        },
        }) {
            name
            state
      }
    }
  }
}

@tthvo tthvo added the fix label Mar 14, 2023
@tthvo tthvo requested review from andrewazores and maxcao13 March 14, 2023 17:18
@tthvo
Copy link
Member Author

tthvo commented Mar 14, 2023

Unfortunately, the label key does not accept forward slash /... So, I am thinking of using a dot separate the last part.

andrewazores
andrewazores previously approved these changes Mar 14, 2023
@tthvo
Copy link
Member Author

tthvo commented Mar 14, 2023

Opps forgot to update tests. Looking into it now.

@andrewazores andrewazores dismissed their stale review March 14, 2023 17:38

Looks like itests need updating

@tthvo tthvo force-pushed the start-rec-mutator branch from da0db80 to 4e0acd3 Compare March 14, 2023 18:08
Signed-off-by: Thuan Vo <thvo@redhat.com>
@tthvo tthvo force-pushed the start-rec-mutator branch from 4e0acd3 to 598d15e Compare March 14, 2023 18:12
@andrewazores
Copy link
Member

Test fix makes sense.

@tthvo
Copy link
Member Author

tthvo commented Mar 14, 2023

Error:  Failures: 
Error:    JmxSessionAuthIT.checkStatusForRecordingsQueryWithoutCredentials:121 
Expected: <427>
     but: was <200>

I think it is related to the thread local issue that you mentioned?

@andrewazores
Copy link
Member

Yea, that's the same bug.

@github-actions
Copy link
Contributor

Test image available:

$ CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat:pr-1411-598d15e8823578d9f8326421a506007666955325 sh smoketest.sh

@tthvo
Copy link
Member Author

tthvo commented Mar 14, 2023

Nice all passed now :D

@andrewazores andrewazores merged commit ef6b6bd into cryostatio:main Mar 14, 2023
@tthvo tthvo deleted the start-rec-mutator branch March 14, 2023 19:02
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] GraphQL mutator for starting recordings does not accept the same "labels" format as the metadata mutator
2 participants