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

(PDB-1072) Add endpoint to mirror report command structure. #1193

Merged

Conversation

Projects
None yet
6 participants
@wkalt
Copy link
Contributor

commented Dec 26, 2014

This commit adds an endpoint called event-reports, the return value of which
can be easily munged for resubmission as a report. In an HA context, this will
allow us to transfer reports from one database to another over HTTP without
needing a second query for the events associated with a report.

@wkalt

This comment has been minimized.

Copy link
Contributor Author

commented Dec 26, 2014

open to suggestions on renaming this endpoint

hash, puppet_version, receive_time, report_format,
start_time, end_time, transaction_uuid, status, environment,
configuration_version, certname FROM (%s) distinct_names %s%s%s) %s"
sql sql inner-order-by limit-clause offset-clause order-by-clause)

This comment has been minimized.

Copy link
@wkalt

wkalt Dec 26, 2014

Author Contributor

need to think about whether we have better options than this pattern. The issue is limit/offset on API responses that comprise multiple rows.

INNER JOIN resource_events re on reports.hash=re.report
LEFT OUTER JOIN environments on reports.environment_id = environments.id
LEFT OUTER JOIN report_statuses on reports.status_id = report_statuses.id
ORDER BY hash"}))

This comment has been minimized.

Copy link
@wkalt

wkalt Dec 26, 2014

Author Contributor

I am not positive that this ordering is necessary.

@pljenkinsro

This comment has been minimized.

Copy link

commented Dec 26, 2014

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/321/

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch from e8bf0b5 to 019ec8b Dec 26, 2014

@pljenkinsro

This comment has been minimized.

Copy link

commented Dec 27, 2014

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/322/

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch 2 times, most recently from 2518125 to 6dedcea Dec 27, 2014

@pljenkinsro

This comment has been minimized.

Copy link

commented Dec 27, 2014

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/324/

@pljenkinsro

This comment has been minimized.

Copy link

commented Dec 27, 2014

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/325/

[puppetlabs.puppetdb.middleware :refer [verify-accepts-json validate-query-params
wrap-with-paging-options]]
[puppetlabs.puppetdb.jdbc :as jdbc]
[puppetlabs.puppetdb.http :as http]))

This comment has been minimized.

Copy link
@ajroetker

ajroetker Jan 5, 2015

Contributor

Are all these dependencies necessary?

@kbarber

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2015

So here is the thing, this new endpoint looks almost exactly like /reports ... but without the downstream information like events ... I'm wondering if we should just call this /reports instead, and let people use the extract field selector capability if they want the old report format? That would save us from having to create a brand new end-point.

@wkalt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2015

that's fine with me in concept, but we'll have to extend extract to cover non-queryable fields, since the event-reports field of this endpoint is not queryable. Not sure what that will take; I'll give it some thought.

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch from 6dedcea to fd3602d Jan 5, 2015

@wkalt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2015

actually, that's not going to be an issue in the slightest. Happy to take this route.

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 6, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/346/

@puppetcla

This comment has been minimized.

Copy link

commented Jan 6, 2015

CLA signed by all contributors.

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch 2 times, most recently from 57972b7 to f8785fe Jan 7, 2015

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/360/

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch 3 times, most recently from fe7a9aa to 1f15228 Jan 7, 2015

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/361/

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/362/

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch from 1f15228 to 969f545 Jan 7, 2015

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/363/

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch from 969f545 to 85acb27 Jan 7, 2015

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/364/

@wkalt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2015

@pljenkinsro retest this please

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/365/

@kbarber

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2015

@pljenkinsro retest this please

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/367/

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch from 85acb27 to e1479b9 Jan 7, 2015

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/373/

@wkalt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2015

@pljenkinsro retest this please

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/374/

@wkalt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2015

@pljenkinsro retest this please

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/375/

@wkalt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2015

@pljenkinsro retest this please

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/376/

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch from e1479b9 to 3ffb104 Jan 7, 2015

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/378/

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch from 3ffb104 to 973e08f Jan 7, 2015

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 7, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/379/

@wkalt wkalt removed the work in progress label Jan 7, 2015

@wkalt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2015

@pljenkinsro retest this please

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 8, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/381/


(defn remove-receive-times
(defn treat-reports-response

This comment has been minimized.

Copy link
@senior

senior Jan 8, 2015

Contributor

Can you come up with a more descriptive function name here? Maybe munge-for-compare, or prep-for-compare or something like that?

[clj-time.core :refer [now ago days]]))

(use-fixtures :each with-test-db)

(defn reports-equal
[r1 r2]
(let [norm (fn [x] (sort-by :hash (map #(update-in % [:resource-events] munge-resource-events) x)))]

This comment has been minimized.

Copy link
@senior

senior Jan 8, 2015

Contributor

Have you seen this fail? I think it will be opaque in that it will just say reports-equal is false. To diagnose the failure you'll have to go in and change the test to give more information (i.e. println here or something). I think if you take your anon-function above, give it a name and then use it directly below like:

(is (= (order-events expected) (order-events actual)))

You'll see what failed



(def data-seq (-> (slurp "./test-resources/puppetlabs/puppetdb/cli/export/reports-query-rows.json")
(json/parse-string)

This comment has been minimized.

Copy link
@senior

senior Jan 8, 2015

Contributor

no need for parens here and below

This comment has been minimized.

Copy link
@wkalt

wkalt Jan 8, 2015

Author Contributor

@senior fixed your comments -- not sure why this one isn't updating

(PDB-1072) Add endpoint to mirror report command structure.
This commit changes the /v4/reports endpoint to mirror the store-report command
structure -- this amounts to an additional field containing resource-events.
The upshot of this is that the api response can be easily munged for resubmission
as a report. In an HA context, this will allow us to transfer reports from one
database to another over HTTP without needing a second query for the events
associated with a report.

@wkalt wkalt force-pushed the wkalt:ticket/master/pdb-1072-symmetric-reports branch from 973e08f to 5b015a2 Jan 8, 2015

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 8, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/383/

@wkalt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2015

@pljenkinsro retest this please

@pljenkinsro

This comment has been minimized.

Copy link

commented Jan 8, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.puppetlabs.com/job/platform_puppetdb_intn-sys_pr/384/

@senior

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2015

+1 from me

senior added a commit that referenced this pull request Jan 8, 2015

Merge pull request #1193 from wkalt/ticket/master/pdb-1072-symmetric-…
…reports

(PDB-1072) Add endpoint to mirror report command structure.

@senior senior merged commit 09cc3ef into puppetlabs:master Jan 8, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.