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

Improve SchedulerClient compliance with Scheduler API #2736

Merged
merged 12 commits into from Dec 30, 2016
Merged

Improve SchedulerClient compliance with Scheduler API #2736

merged 12 commits into from Dec 30, 2016

Conversation

fviale
Copy link
Member

@fviale fviale commented Nov 27, 2016

  • improve the handling of job and task results handling, to allow transparent navigation (job to task, logs, etc). Added correct dozer mappings for job and task results
  • improve the handling of job info and corrected the metrics which were broken in both Java and Rest apis
  • implemented a few previously unsupported methods in SchedulerClient class
  • better handling of the task logs and exception in Task result
  • Added a test in SchedulerClientTest to test a few accesses to the scheduler api, mostly job result but also job state, job info

- improve the handling of job and task results handling, to allow transparent navigation (job to task, logs, etc). Added correct dozer mappings for job and task results
- improve the handling of job info and corrected the metrics which were broken in both Java and Rest apis
- implemented a few previously unsupported methods in SchedulerClient class
- better handling of the task logs and exception in Task result
- Added a test in SchedulerClientTest to test a few accesses to the scheduler api, mostly job result but also job state, job info
throw new UnsupportedOperationException();
Map<String, TaskStateData> taskStateMap = jobStateData.getTasks();
Map<TaskId, TaskState> hmTasks = new HashMap<>();
for (TaskStateData ts : taskStateMap.values()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you call ts "taskStateData" ?

@@ -68,7 +70,13 @@ private void copyGenericInformation(JobStateData jobStateData) {

@Override
public Map<TaskId, TaskState> getHMTasks() {
throw new UnsupportedOperationException();
Map<String, TaskStateData> taskStateMap = jobStateData.getTasks();
Map<TaskId, TaskState> hmTasks = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

can you give a better name to hmTasks ?

Copy link
Contributor

@marcocast marcocast left a comment

Choose a reason for hiding this comment

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

looks ok to me, some naming to refactor, but seems fine. Have you tried to run all the system tests against your local scheduler with these changes?

@sonarqube-activeeon
Copy link

SonarQube analysis reported 5 issues

  1. MAJOR SchedulerRestInterface.java#L1587: Refactor this method to throw at most one checked exception instead of: org.ow2.proactive_grid_cloud_portal.scheduler.exception.NotConnectedRestException, org.ow2.proactive_grid_cloud_portal.scheduler.exception.PermissionRestException rule
  2. MAJOR TaskResultImpl.java#L91: Either log or rethrow this exception. rule
  3. MINOR TaskLogsData.java#L35: Rename this package name to match the regular expression '^[a-z]+(.[a-z][a-z0-9])$'. rule
  4. MINOR TaskRestException.java#L35: Rename this package name to match the regular expression '^[a-z]+(.[a-z][a-z0-9])$'. rule
  5. MINOR TaskResultImpl.java#L89: Immediately return this expression instead of assigning it to the temporary variable "unserializedException". rule

@fviale fviale added this to the 7.23.0 milestone Dec 30, 2016
@fviale fviale merged commit 3109091 into ow2-proactive:master Dec 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants