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.

Sign up for GitHub

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

Jump to bottom

Fix runner to string inconsistency #575

Merged
merged 3 commits into from
Mar 27, 2020
Merged

Fix runner to string inconsistency #575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc6ba0e
Changed Runner.getName() to Runner.toString() when passing to Job.run…
Mar 27, 2020
040780a
Document how & when Runner.toString() or Runner.getName() should be used
Mar 27, 2020
bb8a3ce
Convert getName() to toString(). Use name() for Job.runner.
Mar 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions core/src/main/java/feast/core/job/JobUpdateTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ private Job startJob(
new Job(
jobId,
"",
jobManager.getRunnerType().toString(),
jobManager.getRunnerType().name(),
Source.fromProto(source),
Store.fromProto(sinkSpec),
featureSets,
Expand All @@ -155,7 +155,7 @@ private Job startJob(
jobId,
Action.SUBMIT,
"Building graph and submitting to %s",
jobManager.getRunnerType().getName());
jobManager.getRunnerType().toString());

job = jobManager.startJob(job);
if (job.getExtId().isEmpty()) {
Expand All @@ -168,7 +168,7 @@ private Job startJob(
jobId,
Action.STATUS_CHANGE,
"Job submitted to runner %s with ext id %s.",
jobManager.getRunnerType().getName(),
jobManager.getRunnerType().toString(),
job.getExtId());

return job;
Expand All @@ -179,7 +179,7 @@ private Job startJob(
jobId,
Action.STATUS_CHANGE,
"Job failed to be submitted to runner %s. Job status changed to ERROR.",
jobManager.getRunnerType().getName());
jobManager.getRunnerType().toString());

job.setStatus(JobStatus.ERROR);
return job;
Expand All @@ -206,7 +206,7 @@ private Job updateJob(
Action.UPDATE,
"Updating job %s for runner %s",
job.getId(),
jobManager.getRunnerType().getName());
jobManager.getRunnerType().toString());
return jobManager.updateJob(job);
}

Expand Down
10 changes: 8 additions & 2 deletions core/src/main/java/feast/core/job/Runner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,19 @@ public enum Runner {
this.name = name;
}

public String getName() {
/**
* Get the human readable name of this runner. Returns a human readable name of the runner that
* can be used for logging/config files/etc.
*/
@Override
public String toString() {
return name;
}

/** Parses a runner from its human readable name. */
public static Runner fromString(String runner) {
for (Runner r : Runner.values()) {
if (r.getName().equals(runner)) {
if (r.toString().equals(runner)) {
return r;
}
}
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void abortJob(String dataflowJobId) {
*/
@Override
public JobStatus getJobStatus(Job job) {
if (!Runner.DATAFLOW.getName().equals(job.getRunner())) {
if (!Runner.DATAFLOW.name().equals(job.getRunner())) {
return job.getStatus();
}

Expand Down Expand Up @@ -191,7 +191,7 @@ private Job submitDataflowJob(
.map(
fsp -> {
FeatureSet featureSet = new FeatureSet();
featureSet.setName(fsp.getSpec().getName());
featureSet.setName(fsp.getSpec().toString());
featureSet.setVersion(fsp.getSpec().getVersion());
featureSet.setProject(new Project(fsp.getSpec().getProject()));
return featureSet;
Expand All @@ -201,7 +201,7 @@ private Job submitDataflowJob(
return new Job(
jobName,
jobId,
getRunnerType().getName(),
getRunnerType().name(),
Source.fromProto(source),
Store.fromProto(sink),
featureSets,
Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/feast/core/model/Job.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class Job extends AbstractTimestampEntity {
private String extId;

// Runner type
// Use Runner.name() when converting a Runner to string to assign to this property.
@Column(name = "runner")
private String runner;

Expand Down
18 changes: 9 additions & 9 deletions core/src/test/java/feast/core/job/JobUpdateTaskTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public void shouldUpdateJobIfPresent() {
new Job(
"job",
"old_ext",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1)),
Expand All @@ -119,7 +119,7 @@ public void shouldUpdateJobIfPresent() {
new Job(
"job",
"old_ext",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1), FeatureSet.fromProto(featureSet2)),
Expand All @@ -129,7 +129,7 @@ public void shouldUpdateJobIfPresent() {
new Job(
"job",
"new_ext",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
Source.fromProto(source),
Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1), FeatureSet.fromProto(featureSet2)),
Expand Down Expand Up @@ -163,7 +163,7 @@ public void shouldCreateJobIfNotPresent() {
new Job(
"job",
"",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1)),
Expand All @@ -173,7 +173,7 @@ public void shouldCreateJobIfNotPresent() {
new Job(
"job",
"ext",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1)),
Expand Down Expand Up @@ -202,7 +202,7 @@ public void shouldUpdateJobStatusIfNotCreateOrUpdate() {
new Job(
"job",
"ext",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1)),
Expand All @@ -216,7 +216,7 @@ public void shouldUpdateJobStatusIfNotCreateOrUpdate() {
new Job(
"job",
"ext",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
Source.fromProto(source),
Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1)),
Expand Down Expand Up @@ -248,7 +248,7 @@ public void shouldReturnJobWithErrorStatusIfFailedToSubmit() {
new Job(
"job",
"",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1)),
Expand All @@ -258,7 +258,7 @@ public void shouldReturnJobWithErrorStatusIfFailedToSubmit() {
new Job(
"job",
"",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1)),
Expand Down
4 changes: 2 additions & 2 deletions core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public void shouldStartJobWithCorrectPipelineOptions() throws IOException {
new Job(
jobName,
"",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
Source.fromProto(source),
Store.fromProto(store),
Lists.newArrayList(FeatureSet.fromProto(featureSet)),
Expand Down Expand Up @@ -226,7 +226,7 @@ public void shouldThrowExceptionWhenJobStateTerminal() throws IOException {
new Job(
"job",
"",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
Source.fromProto(source),
Store.fromProto(store),
Lists.newArrayList(FeatureSet.fromProto(featureSet)),
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/feast/core/job/direct/DirectRunnerJobManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public void shouldStartDirectJobAndRegisterPipelineResult() throws IOException {
new Job(
expectedJobId,
"",
Runner.DIRECT.getName(),
Runner.DIRECT.name(),
Source.fromProto(source),
Store.fromProto(store),
Lists.newArrayList(FeatureSet.fromProto(featureSet)),
Expand Down
12 changes: 6 additions & 6 deletions core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public void shouldGenerateAndSubmitJobsIfAny() throws InvalidProtocolBufferExcep
new Job(
"",
"",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1), FeatureSet.fromProto(featureSet2)),
Expand All @@ -171,7 +171,7 @@ public void shouldGenerateAndSubmitJobsIfAny() throws InvalidProtocolBufferExcep
new Job(
"some_id",
extId,
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1), FeatureSet.fromProto(featureSet2)),
Expand Down Expand Up @@ -261,7 +261,7 @@ public void shouldGroupJobsBySource() throws InvalidProtocolBufferException {
new Job(
"name1",
"",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source1),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1)),
Expand All @@ -271,7 +271,7 @@ public void shouldGroupJobsBySource() throws InvalidProtocolBufferException {
new Job(
"name1",
"extId1",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source1),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet1)),
Expand All @@ -281,7 +281,7 @@ public void shouldGroupJobsBySource() throws InvalidProtocolBufferException {
new Job(
"",
"extId2",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source2),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet2)),
Expand All @@ -291,7 +291,7 @@ public void shouldGroupJobsBySource() throws InvalidProtocolBufferException {
new Job(
"name2",
"extId2",
Runner.DATAFLOW.getName(),
Runner.DATAFLOW.name(),
feast.core.model.Source.fromProto(source2),
feast.core.model.Store.fromProto(store),
Arrays.asList(FeatureSet.fromProto(featureSet2)),
Expand Down