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 99 move host #147

Merged
merged 5 commits into from
Sep 12, 2022
Merged

Fix 99 move host #147

merged 5 commits into from
Sep 12, 2022

Conversation

lgetwan
Copy link
Contributor

@lgetwan lgetwan commented Sep 12, 2022

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: #99
When a host was modified and stayed in the same folder, an
Error calling API. HTTP code 400. Details: b'{"title": "Invalid move action", "status": 400, "detail": "The host is already part of the specified target folder"}'
Was raised.

What is the new behavior?

  • The module can now handle changing host attributes while the host remains in the same folder.

Other information

@lgetwan
Copy link
Contributor Author

lgetwan commented Sep 12, 2022

@robin-tribe29: Not sure why the "Tests for roles" are failing.
I fixed #99 with this PR. Can you do a quick review?

@robin-checkmk robin-checkmk changed the base branch from main to devel September 12, 2022 10:08
@robin-checkmk
Copy link
Member

@robin-tribe29: Not sure why the "Tests for roles" are failing. I fixed #99 with this PR. Can you do a quick review?

I am working on new integration tests for roles (#145), so you can ignore the old checks for now.

Copy link
Member

@robin-checkmk robin-checkmk left a comment

Choose a reason for hiding this comment

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

For me host creation still fails with the following error, if the host exists:
fatal: [test9.tld -> localhost]: FAILED! => {"changed": false, "msg": "Error calling API. HTTP code 400. Details: b'{\"title\": \"Invalid move action\", \"status\": 400, \"detail\": \"The host is already part of the specified target folder\"}', "}
Other than that all looks good.

changelogs/fragments/fix-99-move-host.yml Outdated Show resolved Hide resolved
@robin-checkmk robin-checkmk added the bug Something isn't working label Sep 12, 2022
@lgetwan
Copy link
Contributor Author

lgetwan commented Sep 12, 2022

There was another point in the code where a heading "/" was added. Removed that line. Looks good now in my tests.

@robin-checkmk
Copy link
Member

There was another point in the code where a heading "/" was added. Removed that line. Looks good now in my tests.

I'll run a few tests too, but it looks promising now! 🎉
Can you check the open issue, which of them will be solved with this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants