Uploaded image for project: 'REEF (Retired)'
  1. REEF (Retired)
  2. REEF-332 Pass checkstyle
  3. REEF-401

Triage TODO comments in Java code

    XMLWordPrintableJSON

Details

    • Sub-task
    • Status: Resolved
    • Minor
    • Resolution: Duplicate
    • None
    • 0.12
    • All
    • None

    Description

      TodoComment check reports comments which contain a predefined pattern ("TODO")
      To fix the violations, we need two steps:

      1. Go through existing TODO comments and triage them; create and link JIRA bugs/tasks for ones which we intend to fix and delete the ones we don't
      2. Update checkstyle configuration to ignore comments starting with "TODO (JIRA ...)", so that the violation is reported only for untriaged TODOs

      There is a bunch of "TODO: Document" comments, which typically overlap with JavadocType (missing comment) violations - I'll handle them together with javadoc issues in a separate task.
      Please help to triage the rest of TODOs.

      A list of TODOs in our code:

      org/apache/reef/bridge/client/LocalClient.java
      // TODO: Make the parameters of the local runtime command line arguments of this tool.

      org/apache/reef/javabridge/FailedTaskBridge.java
      // TODO: deserialize/serialize with proper Avro schema

      org/apache/reef/javabridge/generic/Launch.java
      // TODO: Remove the injector, have stuff injected.

      org/apache/reef/driver/PreemptionEvent.java
      // TODO: We need to have a set of things to present to the user as preempted. Probably a Set<String> with the Evaluator IDs.

      org/apache/reef/runtime/common/driver/DriverStartHandler.java
      // TODO: We might have to indicate this to YARN somehow such that it doesn't try another time.
      (Driver restart happened, but no ON_DRIVER_RESTART handler is bound.)

      org/apache/reef/runtime/common/driver/evaluator/AllocatedEvaluatorImpl.java
      // TODO: The factories should be removed when deprecated setType is removed, as the process should not be created here

      org/apache/reef/runtime/common/driver/evaluator/EvaluatorManager.java
      // TODO: Eventually remove the factories when they are removed from AllocatedEvaluatorImpl

      org/apache/reef/runtime/common/driver/evaluator/EvaluatorManager.java
      // TODO: currently we obtain the job id directly by parsing execution (container) directory path
      // #845 is open to get the id from RM properly

      org/apache/reef/runtime/common/driver/evaluator/EvaluatorState.java
      // TODO: Add CLOSING state

      org/apache/reef/runtime/common/driver/resourcemanager/ResourceManagerStatus.java
      // TODO: Add state transition check

      org/apache/reef/runtime/common/evaluator/DefaultDriverConnection.java
      // TODO: implement a proper mechanism to obtain driver remote identifier.

      org/apache/reef/runtime/common/evaluator/HeartBeatManager.java
      // TODO: Write a test that checks for the order.

      org/apache/reef/runtime/common/evaluator/context/ContextRuntime.java
      // TODO: Which lock guards this?

      org/apache/reef/runtime/common/launch/LaunchClass.java
      // TODO: This probably should be bound via Tang.

      org/apache/reef/runtime/common/launch/REEFErrorHandler.java
      // TODO: This gets a new EventHandler each time an exception is caught. It would be better to cache the handler. But
      // that introduces threading issues and isn't really worth it, as the JVM typically will be killed once we catch an
      // Exception in here.

      org/apache/reef/runtime/common/launch/parameters/ErrorHandlerRID.java
      // TODO: Find all comparisons with this (NO_ERROR_HANDLER_REMOTE_ID) and see whether we can do something about them

      org/apache/reef/examples/suspend/Launch.java
      // TODO: Remove the injector, have stuff injected.

      org/apache/reef/examples/helloCLR/HelloCLR.java
      // TODO: Make this a config option

      org/apache/reef/io/data/loading/api/DataLoader.java
      // TODO: Add timeouts

      org/apache/reef/io/network/group/impl/operators/BroadcastReceiver.java
      // TODO: Should receive the identity element instead of null

      org/apache/reef/io/network/naming/NameLookupClient.java
      // TODO: better fix uses a map of id's after REEF-198

      org/apache/reef/io/network/naming/NameRegistryClient.java
      // TODO: better fix matches replies to threads with a map after REEF-198

      org/apache/reef/io/network/naming/NameServerImpl.java
      // TODO: All existing NameServer usage is currently new-up, need to make them injected as well.

      org/apache/reef/io/network/util/Utils.java
      // TODO: Merge with parseListCmp() into one generic implementation.

      org/apache/reef/io/storage/local/LocalScratchSpace.java
      // TODO: Error handling

      org/apache/reef/runtime/hdinsight/client/AzureUploader.java
      // TODO: If I don't do this check, the getLength() call below returns 0. No idea why.

      org/apache/reef/runtime/local/driver/ContainerManager.java
      // TODO: Find the actual system memory on this machine.

      org/apache/reef/runtime/mesos/client/MesosJobSubmissionHandler.java
      // The current implementation runs the driver as a local process, similar to reef-runtime-local.
      // TODO: run the driver on a slave node in the cluster

      org/apache/reef/runtime/mesos/driver/MesosResourceLaunchHandler.java
      // TODO: Replace REEFExecutor with a simple launch command (we only need to launch REEFExecutor)

      org/apache/reef/runtime/mesos/driver/REEFScheduler.java
      // Assumes for now that all slaves use port 5051(default) TODO: make it configurable.

      // TODO: make it (user?) configurable.

      // TODO: a hack to pass closeEvaluator test, replace this with a better interface

      // TODO: reflect priority and rack/node locality specified in resourceRequestEvent.

      org/apache/reef/runtime/mesos/evaluator/REEFExecutor.java
      // TODO: a hack to pass closeEvaluator test, replace this with a better interface

      org/apache/reef/runtime/mesos/util/MesosErrorHandler.java
      // ErrorHandler for MesosRemoteManager.
      // TODO: Replace this class once Tang's namespace feature is enabled

      org/apache/reef/runtime/mesos/util/MesosRemoteManager.java
      // Since the existing RemoteManager cannot use an additional codec,
      // we need this additional RemoteManager to use MesosMessageCodec.
      // TODO: Replace this class once Tang's namespace feature is enabled

      org/apache/reef/runtime/yarn/client/YarnJobSubmissionHandler.java
      // TODO: Revisit this. We also have a named parameter for the queue in YarnClientConfiguration.

      org/apache/reef/runtime/yarn/client/YarnSubmissionHelper.java
      // TODO: this is currently being developed on a hacked 2.4.0 bits, should be 2.4.1

      org/apache/reef/runtime/yarn/client/uploader/JobUploader.java
      // TODO: This really should be configurable, but wasn't in the code I moved as part of REEF-228

      org/apache/reef/runtime/yarn/driver/YarnContainerManager.java
      // TODO: return actual values for progress

      // TODO: this is currently being developed on a hacked 2.4.0 bits, should be 2.4.1

      // TODO: revisit this when implementing locality-strictness (i.e. a specific rack request can be ignored)

      // TODO: check vcores once YARN-2380 is resolved

      org/apache/reef/tests/fail/driver/FailDriver.java
      // TODO: notify client?

      org/apache/reef/tang/Configuration.java
      // TODO: Should this return Set<ConstructorDef<T>> instead?

      // TODO: This API seems wonky. Why not return a Set<NamedParameterNode<Set<?>>> instead, and let the caller invoke getBoundSet() above?

      org/apache/reef/tang/formats/AvroConfigurationSerializer.java
      // TODO: This method should implement list deserialization. Implement it when C# side is ready.
      // TODO: This method should implement list serialization. Implement it when C# side is ready.

      org/apache/reef/tang/formats/CommandLine.java
      // TODO: Check for conflicting options.

      org/apache/reef/tang/formats/ConfigurationModuleBuilder.java
      // TODO: It would be nice if this incorporated d by reference so that static analysis / documentation tools
      // could document the dependency between c and d.

      org/apache/reef/tang/implementation/ConfigurationBuilderImpl.java
      // TODO: None of these should be public! - Move to configurationBuilder. Have
      // that wrap itself
      // in a sane Configuration interface...
      // TODO: Should be final again!

      org/apache/reef/tang/implementation/Constructor.java
      // TODO: use ArrayList internally (and maybe for input, too).

      org/apache/reef/tang/implementation/Subplan.java
      // TODO: use ArrayList internally (and maybe for input, too).

      org/apache/reef/tang/implementation/java/ClassHierarchyImpl.java
      // TODO: When handling sets, need to track target of generic parameter, and check the type here!

      org/apache/reef/tang/implementation/java/InjectorImpl.java
      // TODO: We should be insisting on injection futures here! .forkInjector();

      org/apache/reef/tang/implementation/java/InjectorImpl.java
      // TODO: Must be named parameter node. Check.

      org/apache/reef/tang/implementation/java/JavaNodeFactory.java
      // TODO: When we use paramTypes below, we strip generic parameters. Is that OK?

      org/apache/reef/tang/util/ReflectionUtilities.java
      // TODO: Float and double are currently coercible to int and long. This is a bug.

      org/apache/reef/tang/util/Tint.java
      // TODO: Documentation string?

      org/apache/reef/wake/IdentifierParser.java
      // TODO: Modify tang to allow this to use a factory pattern.

      org/apache/reef/wake/impl/BlockingEventHandler.java
      // TODO: a queue is likely overly conservative given that we only need
      // to preserve order of those pairs of events that didn't race (have an ordering)

      // FIXME: There is a race here where the person draining the events might
      // not include their event as the last one. I'm going to assume this does not
      // matter, since all events will still be drained exactly once by someone in
      // the proper order

      // TODO: a non-locking implementation will simply atomically update the head of the
      // queue to index=expectedSize, so that the drainer may drain without synchronization

      org/apache/reef/wake/impl/ForkPoolStage.java
      // TODO: should WakeSharedPool register its stages?

      org/apache/reef/wake/impl/WakeSharedPool.java
      // TODO: need to pass this upwards to REEF can grab it

      org/apache/reef/wake/rx/impl/TimeoutSubject.java
      // TODO: change Subject to specify conversion to T

      Attachments

        Issue Links

          Activity

            People

              MariiaMykhailova Mariia Mykhailova
              MariiaMykhailova Mariia Mykhailova
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: