search

vatesfr / terraform-provider-xenorchestra

Xen Orchestra provider for Terraform
MIT License
150 stars 32 forks source link

WIP: Add hosts data source #126

Closed gohumble closed 3 years ago

gohumble commented 3 years ago

Hello @ddelnano,

this PR should solve #122. This is still WIP. TODO: sorting This commit also found a way into the PR.

Should I revert it? Or just resolve conflicts?

ddelnano commented 3 years ago

Before you edited the PR description you mentioned that there was a bug in #120? That was released in v0.15.0 but I forgot to put it in the release notes (which are updated now).

Should I revert it? Or just resolve conflicts? Whatever is easiest. I'm not too opinionated about it since when it's ready we will squash and merge the PR.

gohumble commented 3 years ago

Thanks @ddelnano. Tested your branch and cherry-picked into this PR. Error seems to be fixed 👍🏽 I may switch to client side filtering and add some sorting to this. After that this should be ready to merge.

ddelnano commented 3 years ago

@gohumble apologies for missing your latest updates. I'll try to give this a look tomorrow.

gohumble commented 3 years ago

@ddelnano never mind. Just added sorting. Faced some issues when trying to filter on the client side. So I kept the server side filtering. Maybe you have a clue on that. Have tested this within my development env. Seems to work just fine.

Still WIP because i wanted to test this a little more and still have to add some docs.

ddelnano commented 3 years ago

I think we should close this PR in favor of #142. I took this branch as a base and improved the acceptance tests. Thank you so much for all your work on this and I would appreciate of review of the other PR.