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

Feature/dns sorting 3507 #4994

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kroseneg
Copy link

@kroseneg kroseneg commented Feb 6, 2023

This adds an option to the service grid to sort and group the hosts by the DNS hierarchy of the host name.
See #3507 .

@cla-bot
Copy link

cla-bot bot commented Feb 6, 2023

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @kroseneg

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please contact us if you think this is the case.

  • If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.

@kroseneg
Copy link
Author

kroseneg commented Feb 6, 2023 via email

@decosilo
Copy link

decosilo commented Feb 7, 2023

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Feb 7, 2023

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @kroseneg

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please contact us if you think this is the case.

  • If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.

@bobapple
Copy link
Member

bobapple commented Feb 8, 2023

@cla-bot check

@cla-bot cla-bot bot added the cla/signed label Feb 8, 2023
@nilmerg
Copy link
Member

nilmerg commented Feb 8, 2023

Hi,

I don't think we will merge this for the following reasons:

  • It has a massive disadvantage because it only has access to a partial result returned by the database. Only a subset of so called pages will be ordered, though there may be more data on previous/next pages which would influence the shown result.
  • Such a functionality won't be added here anymore. Icinga DB Web already supports sorting by multiple columns. The suggestion made here is already possible by using such a sort parameter: ?sort=host.vars.sub1,host.vars.sub0,host.vars.domain
    • A widget in the UI allowing to assemble such a rule more easily is already planned

Though, thanks for your contribution and effort anyway. It just would have been a good idea to ask whether such an implementation is worth taking on 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants