Skip to content

Conversation

miguelHx
Copy link
Contributor

@miguelHx miguelHx commented Jun 29, 2018

This is useful if your database uses true/false for state and want to make prometheus alerts based on that.
Before, booleans were not able to be parsed. See issue #201

@miguelHx miguelHx changed the title Update dbToFloat64 and dbToString methods to support boolean data fro… Support for boolean data types as metrics Jun 29, 2018
@wrouesnel
Copy link
Contributor

This looks like it has linter errors.

@miguelHx
Copy link
Contributor Author

Ok, will work on it right now.

@miguelHx
Copy link
Contributor Author

@wrouesnel checks passed, ready to merge whenever you are ready.

@coveralls
Copy link

coveralls commented Jul 16, 2018

Coverage Status

Coverage increased (+0.4%) to 62.284% when pulling a3cd819 on miguelHx:update_dbtofloat_bool into 07dd31d on wrouesnel:master.

@miguelHx
Copy link
Contributor Author

miguelHx commented Jul 17, 2018

It seems that the functions dbToFloat64 and dbToString are not covered by tests. Since this is the case, how would I increase coverage from coveralls, given that the changes I made are in those functions?

@miguelHx
Copy link
Contributor Author

It doesn't look like the CI error was introduced by my change. @wrouesnel

0.01s$ $HOME/gopath/bin/goveralls -coverprofile=cover.out -service=travis-ci
Error parsing coverage: open cover.out: no such file or directory
The command "$HOME/gopath/bin/goveralls -coverprofile=cover.out -service=travis-ci" exited with 1.

@wrouesnel
Copy link
Contributor

wrouesnel commented Sep 22, 2018

@miguelHx Generally that happens when the smoke-tests fail in some way. But I'll take a look!

@miguelHx

cmd/postgres_exporter/postgres_exporter.go:1::warning: file is not goimported (goimports)
cmd/postgres_exporter/postgres_exporter.go:1::warning: file is not gofmted with -s (gofmt)

(I should look into getting the build to put metalinter alerts on stderr or in a better color)

@miguelHx
Copy link
Contributor Author

miguelHx commented Sep 24, 2018

@wrouesnel I ran the command go run mage.go -v all on my local machine. I did not get the linter warnings from the travis build. Could you trigger another build with a fresh pull? I'm thinking it might be a false positive.

@miguelHx
Copy link
Contributor Author

@wrouesnel can I get some eyes on this? I see some parsing errors in the build log but I don't see how that relates to my code updates.

I think this is what's preventing a successful build (near the top of the build log):

The command "./gh-assets-clone.sh" exited with 1.

I can't find that particular script.

Any help would be appreciated.

@miguelHx
Copy link
Contributor Author

Found the script, going to take a look.

@miguelHx
Copy link
Contributor Author

@wrouesnel I'm looking at the script:

#!/bin/bash
# Script to setup the assets clone of the repository using GIT_ASSETS_BRANCH and
# GIT_API_KEY.

[ ! -z "$GIT_ASSETS_BRANCH" ] || exit 1
[ ! -z "$GIT_API_KEY" ] || exit 1

setup_git() {
  git config --global user.email "travis@travis-ci.org" || exit 1
  git config --global user.name "Travis CI" || exit 1
}

# Constants
ASSETS_DIR=".assets-branch"

# Clone the assets branch with the correct credentials
git clone --single-branch -b "$GIT_ASSETS_BRANCH" \
    "https://${GIT_API_KEY}@github.com/${TRAVIS_REPO_SLUG}.git" "$ASSETS_DIR" || exit 1

Do I need to update something in my branch to make this work? I'm a little lost lol.

@miguelHx miguelHx closed this Nov 14, 2018
@miguelHx miguelHx reopened this Nov 14, 2018
@miguelHx
Copy link
Contributor Author

miguelHx commented Nov 15, 2018

Added some print statements in the script so narrow down where the script is exiting.
The script seems to exit here:
[ ! -z "$GIT_API_KEY" ] || exit 1
-z returns true if the string in $GIT_API_KEY is null. The script wants to continue if the string is NOT null.
Perhaps the $GIT_API_KEY variable is null, this is an issue not introduced by my code changes.

@miguelHx
Copy link
Contributor Author

miguelHx commented Nov 15, 2018

As per my issue #235, now the build passes after the small gh-assets-clone.sh change.
Now the question is: what to do with the 'gh-assets-clone.sh' file?

@miguelHx
Copy link
Contributor Author

triggering an external build to test out latest change

@miguelHx miguelHx closed this Nov 17, 2018
@miguelHx miguelHx reopened this Nov 17, 2018
@miguelHx
Copy link
Contributor Author

miguelHx commented Dec 9, 2018

@wrouesnel I think this PR is ready to merge now :)

@miguelHx
Copy link
Contributor Author

There are many commits in this PR, so if you could click the option to merge all of this as one commit, that would be great.

@miguelHx
Copy link
Contributor Author

@wrouesnel Will this be merged any time soon? Are there any other issues with this PR?:)

@wrouesnel wrouesnel force-pushed the update_dbtofloat_bool branch from 6319433 to a477f26 Compare January 23, 2019 08:53
@wrouesnel
Copy link
Contributor

Ah right, it needs a rebase. I was meaning to click the merge button a while back. I've pushed a best guess at the rebase back to the PR, we'll see if it builds properly...

@miguelHx
Copy link
Contributor Author

@wrouesnel Okay, looks like it built properly. Let me know if any concerns arise.

@miguelHx
Copy link
Contributor Author

@wrouesnel Is the rebase complete?

@miguelHx
Copy link
Contributor Author

@wrouesnel my apologies, I realize that I should do the rebase. I haven't done a rebase before, so I'll do some research and I'll make it happen.

@miguelHx miguelHx force-pushed the update_dbtofloat_bool branch 2 times, most recently from 11748c4 to 9b3feaa Compare March 31, 2019 23:27
@miguelHx miguelHx force-pushed the update_dbtofloat_bool branch from 9b3feaa to cc168f3 Compare March 31, 2019 23:29
@miguelHx
Copy link
Contributor Author

squashed my commits and rebased onto the upstream master. Build failed, I'm worried that my rebase may have had something to do with it :0, but could be a different issue

@miguelHx miguelHx force-pushed the update_dbtofloat_bool branch from 04b7d65 to cc168f3 Compare April 1, 2019 00:06
This is useful if your database uses true/false for state and want to make prometheus alerts based on that.
Before, booleans were not able to be parsed.  See issue prometheus-community#201
@miguelHx miguelHx force-pushed the update_dbtofloat_bool branch from d64651a to a3cd819 Compare June 17, 2019 00:31
@miguelHx
Copy link
Contributor Author

Rebased. I think it's ready to be merged :)

@miguelHx
Copy link
Contributor Author

Friendly reminder :) @wrouesnel

@wrouesnel wrouesnel merged commit 2b896ea into prometheus-community:master Jun 30, 2019
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