Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Update Plugin RDT to supported level #17

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

Conversation

MrDarthShoe
Copy link

@MrDarthShoe MrDarthShoe commented Sep 8, 2017

Updated collector to newest version of snap-plugin-lib-cpp


This change is Reviewable

@@ -336,25 +336,29 @@ void test_collected_metrics_content(PQOSMock *p_mock, pqos_cap *p_cap, unsigned
// Start collecting all metrics
rdt::Collector *rdt = new rdt::Collector(p_mock);
auto metrics = fixture_metrics(num_cores);
EXPECT_NO_THROW(rdt->collect_metrics(metrics));
auto mts = rdt->collect_metrics(metrics);
// /EXPECT_NO_THROW(rdt->collect_metrics(metrics));

Choose a reason for hiding this comment

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

commented out code

@MrDarthShoe MrDarthShoe force-pushed the rdt-supporting branch 11 times, most recently from 2206536 to 593c099 Compare September 18, 2017 12:25
work in progress- testing
@MrDarthShoe MrDarthShoe force-pushed the rdt-supporting branch 2 times, most recently from a7e487a to 352e2d8 Compare September 19, 2017 13:26
@MrDarthShoe
Copy link
Author

Here is link for easier review: https://reviewable.io/reviews/intelsdi-x/snap-plugin-collector-rdt/17#-

RUN yum install -y git cmake mc tmux autoconf automake libtool curl make unzip wget clang gcc-c++
FROM ubuntu:xenial

RUN apt-get update && apt-get upgrade -yq
Copy link
Contributor

Choose a reason for hiding this comment

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

remove double spaces

{
auto metric_ns = extract_ns(metrics[i]);
std::string metric_ns = "/";
metric_ns += mts[i].ns().get_string();
Copy link
Contributor

Choose a reason for hiding this comment

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

Leading namespace by separator should happen in get_string(), not outside. I submitted an issue to address this change in snap-plugin-lib-cpp - intelsdi-x/snap-plugin-lib-cpp#66. Until it's not resolved in plugin-lib side, it might stay as it is in collector tests.

@IzabellaRaulin IzabellaRaulin assigned skonefal and unassigned skonefal Sep 26, 2017
sudo yum install -y git cmake mc tmux autoconf automake libtool curl make unzip wget clang gcc-c++
cd snap-plugin-collector-rdt
sudo apt-get update && sudo apt-get upgrade -yq
sudo apt-get install g++-4.9 gcc-4.9 protobuf-compiler libprotobuf-dev libprotoc-dev libboost-dev libcppnetlib-dev libspdlog-dev git curl cmake golang-go autoconf libtool ca-certificates unzip software-properties-common golang-go -yq
Copy link
Contributor

Choose a reason for hiding this comment

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

golang-go two times

FROM ubuntu:xenial

RUN apt-get update && apt-get upgrade -yq
RUN apt-get install g++-4.9 gcc-4.9 protobuf-compiler libprotobuf-dev libprotoc-dev libboost-dev libcppnetlib-dev libspdlog-dev git curl cmake golang-go autoconf libtool ca-certificates unzip software-properties-common golang-go -yq
Copy link
Contributor

Choose a reason for hiding this comment

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

golang-go two times

@IzabellaRaulin
Copy link
Contributor

The Travis's outcome seems to be false-positive - there is an error:
image

What is more, I cannot build it (executing ./build.sh) even in the docker container based on provided Dockerfile.

 ./build.sh
CMake Error at /usr/share/cmake-3.5/Modules/FindPackageHandleStandardArgs.cmake:148 (message):
  Could NOT find GTest (missing: GTEST_LIBRARY GTEST_INCLUDE_DIR
  GTEST_MAIN_LIBRARY)
Call Stack (most recent call first):
  /usr/share/cmake-3.5/Modules/FindPackageHandleStandardArgs.cmake:388 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake-3.5/Modules/FindGTest.cmake:205 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  CMakeLists.txt:41 (find_package)

It seems that the procedure of installing GTest does not work as expected.

@skonefal
Copy link
Contributor

:(

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

Successfully merging this pull request may close these issues.

4 participants