From 2f309e8be534896c6c46490d1d5bc006c37c3aec Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Thu, 23 Apr 2020 12:36:34 -0400 Subject: [PATCH 1/4] improvement(snapshots): add preserve stop times flag to avoid normalizing stop_sequence re conveyal/gtfs-lib#283 --- .../datatools/editor/jobs/CreateSnapshotJob.java | 2 +- .../datatools/editor/jobs/ExportSnapshotToGTFSJob.java | 1 + .../conveyal/datatools/manager/models/FeedSource.java | 10 +++++++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/editor/jobs/CreateSnapshotJob.java b/src/main/java/com/conveyal/datatools/editor/jobs/CreateSnapshotJob.java index 317d872f1..fdce4bea3 100644 --- a/src/main/java/com/conveyal/datatools/editor/jobs/CreateSnapshotJob.java +++ b/src/main/java/com/conveyal/datatools/editor/jobs/CreateSnapshotJob.java @@ -87,7 +87,7 @@ public void jobLogic() { Collection existingSnapshots = feedSource.retrieveSnapshots(); int version = existingSnapshots.size(); status.update("Creating snapshot...", 20); - FeedLoadResult loadResult = makeSnapshot(namespace, DataManager.GTFS_DATA_SOURCE); + FeedLoadResult loadResult = makeSnapshot(namespace, DataManager.GTFS_DATA_SOURCE, !feedSource.preserveStopTimes); snapshot.version = version; snapshot.namespace = loadResult.uniqueIdentifier; snapshot.feedLoadResult = loadResult; diff --git a/src/main/java/com/conveyal/datatools/editor/jobs/ExportSnapshotToGTFSJob.java b/src/main/java/com/conveyal/datatools/editor/jobs/ExportSnapshotToGTFSJob.java index 345bc234c..5df370af5 100644 --- a/src/main/java/com/conveyal/datatools/editor/jobs/ExportSnapshotToGTFSJob.java +++ b/src/main/java/com/conveyal/datatools/editor/jobs/ExportSnapshotToGTFSJob.java @@ -52,6 +52,7 @@ public void jobLogic() { FeedLoadResult result = exporter.exportTables(); if (result.fatalException != null) { status.fail(String.format("Error (%s) encountered while exporting database tables.", result.fatalException)); + return; } // Override snapshot ID if exporting feed for use as new feed version. diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java index 27fc41cee..4944c838d 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java @@ -47,9 +47,17 @@ public class FeedSource extends Model implements Cloneable { /** * The collection of which this feed is a part */ - //@JsonView(JsonViews.DataDump.class) public String projectId; + /** + * When snapshotting a GTFS feed for editing, gtfs-lib currently defaults to normalize stop sequence values to be + * zero-based and incrementing. This can muck with GTFS files that are linked to GTFS-rt feeds by stop_sequence, so + * this override flag currently provides a workaround for feeds that need to be edited but do not need to edit + * stop_times or individual patterns. WARNING: enabling this flag for a feed and then attempting to edit patterns in + * complicated ways (e.g., modifying the order of pattern stops) could have unexpected consequences. + */ + public boolean preserveStopTimes; + /** * Get the Project of which this feed is a part */ From 3344ef60baa19b6b5fd21d1d0d96fe76d8dd81c6 Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Thu, 23 Apr 2020 15:47:38 -0400 Subject: [PATCH 2/4] refactor: address PR #304 comments --- .../conveyal/datatools/common/status/MonitorableJob.java | 7 ++++--- .../datatools/editor/jobs/CreateSnapshotJob.java | 2 +- .../datatools/editor/jobs/ExportSnapshotToGTFSJob.java | 9 ++++++--- .../conveyal/datatools/manager/models/FeedSource.java | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/conveyal/datatools/common/status/MonitorableJob.java b/src/main/java/com/conveyal/datatools/common/status/MonitorableJob.java index b5c9654ee..8b1bbee6c 100644 --- a/src/main/java/com/conveyal/datatools/common/status/MonitorableJob.java +++ b/src/main/java/com/conveyal/datatools/common/status/MonitorableJob.java @@ -176,12 +176,13 @@ public void run () { // because the error presumably already occurred and has a better error message. cancel(cancelMessage); } - // Run final steps of job pending completion or error. Note: any tasks that depend on job success should - // check job status to determine if final step should be executed (e.g., storing feed version in MongoDB). - // TODO: should we add separate hooks depending on state of job/sub-tasks (e.g., success, catch, finally) // Complete the job (as success if no errors encountered, as failure otherwise). if (!parentJobErrored && !subTaskErrored) status.completeSuccessfully("Job complete!"); else status.complete(true); + // Run final steps of job pending completion or error. Note: any tasks that depend on job success should + // check job status in jobFinished to determine if final step should be executed (e.g., storing feed + // version in MongoDB). + // TODO: should we add separate hooks depending on state of job/sub-tasks (e.g., success, catch, finally) jobFinished(); // We retain finished or errored jobs on the server until they are fetched via the API, which implies they diff --git a/src/main/java/com/conveyal/datatools/editor/jobs/CreateSnapshotJob.java b/src/main/java/com/conveyal/datatools/editor/jobs/CreateSnapshotJob.java index fdce4bea3..04fe9ae9e 100644 --- a/src/main/java/com/conveyal/datatools/editor/jobs/CreateSnapshotJob.java +++ b/src/main/java/com/conveyal/datatools/editor/jobs/CreateSnapshotJob.java @@ -87,7 +87,7 @@ public void jobLogic() { Collection existingSnapshots = feedSource.retrieveSnapshots(); int version = existingSnapshots.size(); status.update("Creating snapshot...", 20); - FeedLoadResult loadResult = makeSnapshot(namespace, DataManager.GTFS_DATA_SOURCE, !feedSource.preserveStopTimes); + FeedLoadResult loadResult = makeSnapshot(namespace, DataManager.GTFS_DATA_SOURCE, !feedSource.preserveStopTimesSequence); snapshot.version = version; snapshot.namespace = loadResult.uniqueIdentifier; snapshot.feedLoadResult = loadResult; diff --git a/src/main/java/com/conveyal/datatools/editor/jobs/ExportSnapshotToGTFSJob.java b/src/main/java/com/conveyal/datatools/editor/jobs/ExportSnapshotToGTFSJob.java index 5df370af5..95d2de8d2 100644 --- a/src/main/java/com/conveyal/datatools/editor/jobs/ExportSnapshotToGTFSJob.java +++ b/src/main/java/com/conveyal/datatools/editor/jobs/ExportSnapshotToGTFSJob.java @@ -21,6 +21,7 @@ public class ExportSnapshotToGTFSJob extends MonitorableJob { private static final Logger LOG = LoggerFactory.getLogger(ExportSnapshotToGTFSJob.class); private final Snapshot snapshot; private final String feedVersionId; + private File tempFile; public ExportSnapshotToGTFSJob(Auth0UserProfile owner, Snapshot snapshot, String feedVersionId) { super(owner, "Exporting snapshot " + snapshot.name, JobType.EXPORT_SNAPSHOT_TO_GTFS); @@ -40,7 +41,6 @@ public Snapshot getSnapshot () { @Override public void jobLogic() { - File tempFile; try { tempFile = File.createTempFile("snapshot", "zip"); } catch (IOException e) { @@ -72,12 +72,15 @@ public void jobLogic() { status.fail(String.format("Could not store feed for snapshot %s", snapshot.id), e); } } - // Delete snapshot temp file. - tempFile.delete(); } @Override public void jobFinished () { if (!status.error) status.completeSuccessfully("Export complete!"); + // Delete snapshot temp file. + if (tempFile != null) { + LOG.info("Deleting temporary GTFS file for exported snapshot at {}", tempFile.getAbsolutePath()); + tempFile.delete(); + } } } diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java index 4944c838d..45cdbb441 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java @@ -56,7 +56,7 @@ public class FeedSource extends Model implements Cloneable { * stop_times or individual patterns. WARNING: enabling this flag for a feed and then attempting to edit patterns in * complicated ways (e.g., modifying the order of pattern stops) could have unexpected consequences. */ - public boolean preserveStopTimes; + public boolean preserveStopTimesSequence; /** * Get the Project of which this feed is a part From 22ba7cf105ad44180c9a1d42793475404d6b457e Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Thu, 23 Apr 2020 17:09:43 -0400 Subject: [PATCH 3/4] build(deps): bump gtfs-lib to 6.0.2 brings in preserveStopTimes flag for snapshotting --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7df340c57..9e6c98f7c 100644 --- a/pom.xml +++ b/pom.xml @@ -250,7 +250,7 @@ com.conveyal gtfs-lib - 6.0.1 + 6.0.2 From dec1b66ec59b16cc72c4648a17f1ca62a92ff269 Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Tue, 19 May 2020 13:56:24 -0400 Subject: [PATCH 4/4] refactor: adjust comment --- .../java/com/conveyal/datatools/manager/models/FeedSource.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java index 45cdbb441..a305f9584 100644 --- a/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java +++ b/src/main/java/com/conveyal/datatools/manager/models/FeedSource.java @@ -54,7 +54,8 @@ public class FeedSource extends Model implements Cloneable { * zero-based and incrementing. This can muck with GTFS files that are linked to GTFS-rt feeds by stop_sequence, so * this override flag currently provides a workaround for feeds that need to be edited but do not need to edit * stop_times or individual patterns. WARNING: enabling this flag for a feed and then attempting to edit patterns in - * complicated ways (e.g., modifying the order of pattern stops) could have unexpected consequences. + * complicated ways (e.g., modifying the order of pattern stops) could have unexpected consequences. There is no UI + * setting for this and it is not recommended to do this unless absolutely necessary. */ public boolean preserveStopTimesSequence;