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 mssql for non-default sql instances #237

Merged
merged 1 commit into from
Aug 4, 2018
Merged

fix mssql for non-default sql instances #237

merged 1 commit into from
Aug 4, 2018

Conversation

szook
Copy link
Contributor

@szook szook commented Aug 3, 2018

the only sql servers I have access to are using the default
MSSQLSERVER instance names. I was contacted by someone using a
different instance name and it was failing.

Found 2 bugs in the code:

  1. I was returning the default MSSQLSERVER as an instance even if it wasn't found in the registry

  2. learned that the WMI class naming scheme for non-default instances
    was not what I had coded. Changed to incorproate new knowledge.

@szook
Copy link
Contributor Author

szook commented Aug 3, 2018

work in progress until the AccessMethods change is merged and I can rebase.

@carlpett
Copy link
Collaborator

carlpett commented Aug 3, 2018

#236 is merged!

@carlpett
Copy link
Collaborator

carlpett commented Aug 3, 2018

LGTM, anything apart from rebase needed?

the only sql servers I have access to are using the default
`MSSQLSERVER` instance names.  I was contacted by someone using a
different instance name and it was failing.

Found 2 bugs in the code:

1) I was returning the default `MSSQLSERVER` as an instance even if it wasn't found in the registry

2) learned that the WMI class naming scheme for non-default instances
was not what I had coded. Changed to incorproate new knowledge.
@szook
Copy link
Contributor Author

szook commented Aug 3, 2018

I'm also working offline with @sysadmind to see if this, indeed, works.

@szook szook changed the title wip: fix mssql for non-default sql instances fix mssql for non-default sql instances Aug 3, 2018
@szook
Copy link
Contributor Author

szook commented Aug 3, 2018

got confirmation that this works. Removed WIP

Copy link

@sysadmind sysadmind left a comment

Choose a reason for hiding this comment

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

Works for me with a named instance. Thanks @szook !!

@carlpett carlpett merged commit bad1e7f into prometheus-community:master Aug 4, 2018
@carlpett
Copy link
Collaborator

carlpett commented Aug 4, 2018

Thanks! Cutting a new release in a few minutes

@szook szook deleted the fix-mssql-for-non-default-instances branch August 7, 2018 19:37
anubhavg-icpl pushed a commit to anubhavg-icpl/windows_exporter that referenced this pull request Sep 22, 2024
…non-default-instances

fix mssql for non-default sql instances
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