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

adagios-2.0.2-1 #671

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

adagios-2.0.2-1 #671

wants to merge 6 commits into from

Conversation

gardart
Copy link
Contributor

@gardart gardart commented Oct 12, 2021

No description provided.

@@ -23,8 +23,8 @@ Alias /media /opt/adagios/adagios/media
ProxyPass !
</Location>

<Location /adagios/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you update the pull request with information on why this was needed?

It feels like this might cause subtle breakages for other use cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only used if you run Adagios with Gunicorn and Apache. What use cases do you think will brake by using this?

@@ -4,7 +4,6 @@
# If set, adagios will use this file to manage your object
# definitions. If set to None, adagios will search most common
# paths like /etc/nagios/nagios.cfg for it
#nagios_config = "/etc/nagios/nagios.cfg"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is change required? Note the idea of having this commented out by default is for adagios to attempt to discover location of nagios.cfg in common locations and now that functionality becomes broken for default installs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes maybe best to have this commented out by default. I will change this.

@@ -0,0 +1,116 @@
# -*- coding: utf-8 -*-
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't look at the diff between adagios.conf and adagios_naemon.conf but are they significant?

I'm wondering what it would take to support naemon out of the box without having to fork the config file completely (this causes technical debt long term having to keep both config files in sync).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, best solution would be to support Naemon out of the box. I will look further into that.

@sonarcloud
Copy link

sonarcloud bot commented Nov 15, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

None yet

2 participants