-
Notifications
You must be signed in to change notification settings - Fork 124
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
Export jobs #718
Export jobs #718
Conversation
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
Jenkins Build failure 489 tests run, 0 skipped, 2 failed. |
File zipFile = exportFilePath.toFile(); | ||
WriteToZipConsumer zipConsumer = new WriteToZipConsumer(zipFile); | ||
|
||
String[] jobsArray = jobIds != null ? jobsArray = jobIds.split(",") : null; |
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.
does this work?
Shoudn't it be:
String[] jobsArray = jobIds != null ? jobIds.split(",") : null;
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.
oddly, it appears to have worked, but I'll fix it
* Jobs app server, but pointing to the Traces database. | ||
* @return | ||
*/ | ||
private DatabaseClient getTraceDatabaseClient() { |
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 HubConfig.newTraceDbCient()
I put those new*Client() methods there so that we don't duplicate the creation code. Also, it handles SSL and stuff. This approach will likely fail for ssl.
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 I started with that, but the Trace app server uses a non-standard rewriter that doesn't include some hidden URLs. DMSDK failed because of that. We talked about re-copying the rewriter stuff, but that doesn't feel like a good approach long term.
Won't the jobClient.getSecurityContext()
figure out the SSL situation?
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.
that rewriter is moving into the server install dir and will be maintained like the regular rest rewriter. So we can count on it being ok. Erik says there is a magical tool that checks to make sure all the various rewriter*.xml files are in sync when a server build is created.
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.
Okay, cool. I'll update the writer for now and switch to using newTraceDbClient
@@ -38,7 +38,7 @@ abstract class HubTask extends DefaultTask { | |||
|
|||
@Internal | |||
JobManager getJobManager() { | |||
return new JobManager(getHubConfig().newJobDbClient()); | |||
return new JobManager(getHubConfig().newJobDbClient(), getHubConfig().newTraceDbClient()); |
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.
See above comment. This one is correct. it's using newTraceDbClient() from HubConfig
Jenkins Build failure 489 tests run, 0 skipped, 6 failed. |
The tracing-rewriter was missing some internal endpoints, which prevented some DMSDK calls from working.
Added .json/.xml extensions to trace documents.
Jenkins Build failure 484 tests run, 0 skipped, 2 failed. |
Jenkins Build failure 1 tests run, 0 skipped, 1 failed. |
Jenkins Build failure 489 tests run, 0 skipped, 2 failed. |
The two failing tests are TracingTest ones that are passing locally but not in Jenkins. |
@paxtonhare I think I've addressed your review comments. Please re-review |
No description provided.