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

Remove detectors #817

Merged
merged 5 commits into from
Jul 15, 2019
Merged

Remove detectors #817

merged 5 commits into from
Jul 15, 2019

Conversation

ldelossa
Copy link
Contributor

@ldelossa ldelossa commented Jul 3, 2019

Closes milestone: #745

Removes detectors from AncestryGet rpc call. Response is now:

{
  "ancestry": {
    "name": "test-ancestry",
    "layers": [
      {
        "layer": {
          "hash": "8e674ad76dcef6f6d0398bc25550f680f8751876064a87a15347f00687492090"
        }
      }
    ]
  }
}

// The list of layers along with detected features in each.
repeated AncestryLayer layers = 3;
}
// The ancestry requested.
Ancestry ancestry = 1;
// The status of Clair at the time of the request.
ClairStatus status = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

We still need the status so that clients can determine when they need to rescan something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@ldelossa
Copy link
Contributor Author

ldelossa commented Jul 9, 2019

@jzelinskie

  "ancestry": {
    "name": "test-ancestry",
    "layers": [
      {
        "layer": {
          "hash": "8e674ad76dcef6f6d0398bc25550f680f8751876064a87a15347f00687492090"
        }
      }
    ]
  },
  "status": {
    "detectors": [
      {
        "name": "apk",
        "version": "1.0",
        "dtype": "DETECTOR_D_TYPE_FEATURE"
      },
      {
        "name": "dpkg",
        "version": "1.0",
        "dtype": "DETECTOR_D_TYPE_FEATURE"
      },
      {
        "name": "rpm",
        "version": "1.0",
        "dtype": "DETECTOR_D_TYPE_FEATURE"
      },
      {
        "name": "apt-sources",
        "version": "1.0",
        "dtype": "DETECTOR_D_TYPE_NAMESPACE"
      },
      {
        "name": "lsb-release",
        "version": "1.0",
        "dtype": "DETECTOR_D_TYPE_NAMESPACE"
      },
      {
        "name": "os-release",
        "version": "1.0",
        "dtype": "DETECTOR_D_TYPE_NAMESPACE"
      },
      {
        "name": "redhat-release",
        "version": "1.0",
        "dtype": "DETECTOR_D_TYPE_NAMESPACE"
      },
      {
        "name": "alpine-release",
        "version": "1.0",
        "dtype": "DETECTOR_D_TYPE_NAMESPACE"
      }
    ],
    "last_update_time": "2019-07-03T21:41:56.000Z"
  }
}```

updated api output. 

@jzelinskie
Copy link
Contributor

LGTM if you squash the last two commits into one.

@ldelossa
Copy link
Contributor Author

@jzelinskie squashed.

@ldelossa ldelossa force-pushed the remove-detectors branch 2 times, most recently from e325209 to d6364d1 Compare July 12, 2019 20:00
@ldelossa
Copy link
Contributor Author

@jzelinskie should be good to go. Added .clang-format, make targets for proto linting and jsonnet compilation. CI passing.

Copy link
Contributor

@jzelinskie jzelinskie left a comment

Choose a reason for hiding this comment

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

1 final change

Makefile Outdated

.PHONY: gen-drone-config
gen-drone-config:
jsonnet -y .drone.jsonnet -o .drone.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should make this drone jsonnet --stream since that's what their documentation says to use.

@jzelinskie jzelinskie merged commit c9e4e64 into quay:master Jul 15, 2019
@ldelossa ldelossa deleted the remove-detectors branch July 16, 2019 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants