Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixes crashes and problems updating metrics #92

Merged
merged 2 commits into from May 16, 2016

Conversation

Projects
None yet
3 participants

@fche and I were able to fix issues with the Python bindings. These patches fixed the problems on my end.

Contributor

natoscott commented May 15, 2016

@taintedkernel thanks for looking into this. It'd be good to add a regression test or two for this code - do you have some sample python code you've been testing with that you could send through? (I'll add it into the QA suite if so, thanks!)

Contributor

fche commented May 16, 2016

do you have some sample python code?

Try the sample code - commented out - in src/python/pcp/mmv.py

Contributor

natoscott commented May 16, 2016

| Try the sample code - commented out - in src/python/pcp/mmv.py

(this is test case qa/704 and qa/src/test_mmv.py) - I'm more specifically looking for test code that exercises the failing situations here, thanks.

The code in mmv.py mostly works, but I had to make a few minor
modifications to get it to run correctly. I'll be able to send over
tomorrow what I have put together thus far.

On Sun, May 15, 2016 at 5:50 PM, Frank Ch. Eigler notifications@github.com
wrote:

do you have some sample python code?

Try the sample code - commented out - in src/python/pcp/mmv.py


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#92 (comment)

Contributor

natoscott commented May 16, 2016

I'll be able to send over tomorrow what I have put together thus far.

@taintedkernel great, thanks!

Contributor

natoscott commented May 16, 2016

Ah, I also just found that test qa/704 is marked as "reserved" & not actively being run, so it may well become healthy with these fixes & provide the coverage we need - will take a look.

@taintedkernel any other test code you send through will still be helpful and used too though, thanks.

@natoscott natoscott merged commit 65aa5be into performancecopilot:master May 16, 2016

Taking a second look, there's only really 2 issues with the example code in
mmv.py. The typeof and semantics constants weren't defined so I changed
them to reference the PM_ types in cpmapi. Also, the call to
lookup_mapping requires an instance string as the second argument (None
will raise an AttributeError).

Attached is example code with the fixes. I'm putting together a short demo
for my team with this functionality and may be able to also send over that
code afterwards.

On Sun, May 15, 2016 at 9:10 PM, Nathan Scott notifications@github.com
wrote:

Ah, I also just found that test qa/704 is marked as "reserved" & not
actively being run, so it may well become healthy with these fixes &
provide the coverage we need - will take a look.

@taintedkernel https://github.com/taintedkernel any other test code you
send through will still be helpful and used too though, thanks.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#92 (comment)

Contributor

natoscott commented May 16, 2016

| The typeof and semantics constants weren't defined so I changed them to reference the PM_ types in cpmapi

Ah, the commented out sample code is missing an "import cmmv" - I'll fix that up. In fact, maybe a different sample program entirely is in order here.

| Also, the call to lookup_mapping requires an instance string as the second argument (None will raise an AttributeError).

Yes, I came across that one yesterday when working on regressions tests here - so that one's fixed already.

natoscott added a commit that referenced this pull request May 16, 2016

docs: improve the sample code in the python mmv module
Use a slightly more realistic example, as discussed in:
#92
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment