Skip to content

Conversation

@anazobec
Copy link
Collaborator

API module with action get would sometimes return a list of with only a key present instead of dict.
This minor bug could be seen in the records output on calling the api module with action get on endpoint /ping which returned:

record: [
  "status"
]

This PR fixes this bug to by making a minor change in the filter_resultsfunction in module_utils/utils.py.
The return should now look like this:

record: [
  {
    "status": "Active"
  }
]

@anazobec anazobec self-assigned this May 22, 2023
@anazobec anazobec requested a review from justinc1 May 22, 2023 07:25
Copy link
Collaborator

@justinc1 justinc1 left a comment

Choose a reason for hiding this comment

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

I think api module should just return what API does return. I'm note sure how much such "requirement" complicates the work.

You said some integration tests fail - which?

@anazobec anazobec force-pushed the fix-api-module-ping-endpoint-return branch from 5df2646 to d1e799e Compare May 22, 2023 12:33
@anazobec
Copy link
Collaborator Author

Yes, I've mentioned that some integration tests fail. Before the latest commit, 12 tests failed. 9 with error Connection Refused and others with just an assertion failure.

Integ tests with Connection Refused

  • vm_clone
  • virtual_disk_attach
  • virtual_disk
  • version_update__shutdown_restart
  • git_issue_3
  • git_issue_15
  • git_issue_11
  • certificate (both of them)

Integ tests with Assertion failure

  • version_update_info
  • git_issue_41
  • utils_login

The results of these integration tests, can be seen from here: https://github.com/ScaleComputing/HyperCoreAnsibleCollection/actions/runs/5043144438/jobs/9044620677

@anazobec
Copy link
Collaborator Author

anazobec commented May 22, 2023

With the latest commit (at this point d1e799e), API module should now on /ping endpoint, with data { status: "Active" } return something like this:

record: {
  "status": "Active"
}

If data on the endpoint is not a single dictionary, then a list of dictionaries will be returned.

@anazobec
Copy link
Collaborator Author

anazobec commented May 23, 2023

Here are the newly run integration tests as of commit d1e799e: https://github.com/ScaleComputing/HyperCoreAnsibleCollection/actions/runs/5045862848

11 tests fail: 2 are assertion errors, 1 of them is an error caused by changing the get_records function in the api module. The rest of the tests fail with a Connection Refused error, Unexpected response, EOF.

Assertion failure:

  1. version_update_info (.200)
  2. utils_login (.50) => is caused by fixing api module

Connection Refused failure:

  1. version_update__shutdown_restart_vms (.200)
  2. email_alert (.200)
  3. git_issue_15 (.201)
  4. git_issue_11 (.201)
  5. api (.201)
  6. virtual_disk (.201)

Other failures:

  1. certificate (.200, .201) => EOF in violation of protocol (_ssl.c: 997)
  2. git_issue_41 (.200) => Unexpected response - 500

These tests were rerun 3 times, first time ending with about 20 failures.

@justinc1 justinc1 force-pushed the fix-api-module-ping-endpoint-return branch from 34e0c2f to 5170b18 Compare May 23, 2023 06:25
Copy link
Collaborator

@justinc1 justinc1 left a comment

Choose a reason for hiding this comment

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

Thank you, nice work. I did very trivial changes, just small unittest/comment cleanup.

@anazobec anazobec force-pushed the fix-api-module-ping-endpoint-return branch from a6d20fe to 89c16e9 Compare May 23, 2023 10:38
@justinc1 justinc1 force-pushed the fix-api-module-ping-endpoint-return branch from 89c16e9 to 612d0d0 Compare June 1, 2023 13:31
@justinc1 justinc1 merged commit 30b7a71 into main Jun 1, 2023
@justinc1 justinc1 deleted the fix-api-module-ping-endpoint-return branch June 1, 2023 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants