-
Notifications
You must be signed in to change notification settings - Fork 124
Feature/delete old jobs #566
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
Conversation
| this.finalClient = hubConfig.newFinalClient(); | ||
| this.jobClient = hubConfig.newJobDbClient(); | ||
| this.jobManager = new JobManager(this.jobClient); | ||
| this.jobManager = new JobManager(this.jobClient, hubConfig.traceDbName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look at the $config:JOB-DATABASE variable in xquery. You don't need to pass this arround.
import module namespace config = "http://marklogic.com/data-hub/config"
at "/com.marklogic.hub/lib/config.xqy";| import java.util.TimeZone; | ||
|
|
||
| public class JobManager { | ||
| public class JobManager extends ResourceManager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something tells me you didn't want to inherit this here.
| $input as document-node()* | ||
| ) as document-node()* | ||
| { | ||
| xdmp:log("delete-jobs: " || xdmp:quote($params)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please wrap this entry point in:
perf:log('/v1/resources/flow:post', function() {
(: your stuff :)
})| $input as document-node()* | ||
| ) as document-node()* | ||
| { | ||
| xdmp:log("delete-jobs: " || xdmp:quote($params)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, use debug:log instead of xdmp:log. better yet, leave 'em out.
| service:delete-traces($job-ids) | ||
| }, | ||
| <options xmlns="xdmp:eval"> | ||
| <database>{xdmp:database(map:get($params, "tracingDB"))}</database> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this goes back to my first comment at the top of the page. use $config:JOB-DATABASE
|
|
||
| declare option xdmp:mapping "false"; | ||
|
|
||
| declare private function service:delete-jobs($job-ids as xs:string*) as map:map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put these private methods in a com.marklogic.hub/job-lib.xqy to be consistent with the other stuff.
| public static void setupSuite() throws IOException { | ||
| XMLUnit.setIgnoreWhitespace(true); | ||
| File projectDirFile = projectDir.toFile(); | ||
| if (projectDirFile.isDirectory() && projectDirFile.exists()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is a method in HubTestBase to delete the project dir for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh. This was a copy/paste. But I'll look for that method.
| scaffolding.createFlow(ENTITY, "testharmonize", FlowType.HARMONIZE, | ||
| CodeFormat.XQUERY, DataFormat.XML); | ||
|
|
||
| DataHub dh = new DataHub(getHubConfig()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a getDataHub() convenience method for you in HubTestBase
| </mdl-button> | ||
| </td> | ||
| <td class="job-delete-checkbox"> | ||
| <input type="checkbox" id="list-checkbox-{{job.jobId}}" class="mdl-checkbox__input" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out mdl-checkbox: http://mseemann.io/angular2-mdl/toggle
It's prettier.
|
Add a confirmation before deleting the jobs. See the skull button on the Dashboard for an example. |
| let $results := map:map() | ||
| let $_ := | ||
| for $id in $job-ids | ||
| let $uri := cts:uris((), (), cts:json-property-value-query("jobId", $id)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be done like so?
for $uri in cts:uris((), (), cts:json-property-value-query("jobId", $job-ids))There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also using the $id in reporting back in the $results map, so I do need $id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. cool.
| { | ||
| for $id in $job-ids | ||
| let $traces := /trace[jobId = $id]/traceId | ||
| for $trace in $traces |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
silly nitpick.... remove the $traces var
| for $id in $job-ids | ||
| let $traces := /trace[jobId = $id]/traceId | ||
| for $trace in $traces | ||
| let $_ := xdmp:document-delete(fn:base-uri($trace)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's also xdmp:node-uri. I forget... but I think base-uri can have unintended results in some situations. unlikely here.
| assertEquals(0, actual.totalCount); | ||
| assertEquals(1, actual.errorCount); | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just for grins can you do 2 more tests? one with "" as the id param and one with null as the param.
| import com.marklogic.hub.flow.Flow; | ||
| import com.marklogic.hub.flow.FlowType; | ||
| import com.marklogic.quickstart.EnvironmentAware; | ||
| import com.marklogic.quickstart.model.EnvironmentConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this necessary?
| loadingJobs: boolean = false; | ||
| searchResponse: SearchResponse; | ||
| jobs: Array<Job>; | ||
| jobsToDelete: any = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mind typing this to string or whatever it should be? same for the funcs below.
|
|
||
| import com.marklogic.client.DatabaseClient; | ||
| import com.marklogic.client.DatabaseClientFactory; | ||
| import com.marklogic.client.io.StringHandle; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this necessary?
MarkLogic 8.0-7 has a bug regarding transaction mode, which is fixed in 8.0-7.1. Made some changes to get the extension to work right in 8.0-7.
|
The Travis build is broken, but only with that one test we discussed. All others are passing (after building in special cases for 8.0-7 and 9.0-1.1). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only 1 comment
| let $options := | ||
| <options xmlns="xdmp:eval"> | ||
| { | ||
| if (xdmp:version() = "9.0-1.1") then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is 9.0-1.1 the only outlier here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
among the supported versions (8.0-7+ and 9.0-1.1+), yes
| let $results := map:map() | ||
| let $_ := | ||
| for $id in $job-ids | ||
| let $uri := cts:uris((), (), cts:json-property-value-query("jobId", $id)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. cool.
No description provided.