JanusGraph/janusgraph

updateIndex() should make error cases easier to understand

Open

#1,378 opened on Jan 14, 2019

View on GitHub
 (0 comments) (0 reactions) (0 assignees)Java (1,137 forks)batch import
help wantedkind/enhancement

Repository metrics

Stars
 (4,985 stars)
PR merge metrics
 (Avg merge 3d 18h) (12 merged PRs in 30d)

Description

ManagementSystem.updateIndex() returns null if the provided SchemaAction is not applicable for the current status of the index:

if (!updateAction.isApplicableStatus(schemaVertex.getStatus()))
    return null;

Source

This is confusing and the API docs also don't mention this behaviour:

/**
 * Updates the provided index according to the given {@link SchemaAction}
 *
 * @param index
 * @param updateAction
 * @return a future that completes when the index action is done
 */
IndexJobFuture updateIndex(Index index, SchemaAction updateAction);

The method should either throw an exception in this case or at least document this behaviour in the API docs.

When a user doesn't know about this behaviour, then all they're getting is a NullPointerException by calling get() on the returned IndexJobFuture (see #1359).

Contributor guide

Research direction
Analyze the updateIndex method and decide whether to throw an exception for inapplicable SchemaAction or to document the current behavior.
Tech stack
java
Domain
backend
Issue type
Bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
Active
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
Clear
Prerequisites
JavaGit
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
75