Generate Configuration Documentation - Part 1 by t2gran · Pull Request #4478 · opentripplanner/OpenTripPlanner

t2gran · 2022-09-28T17:12:28Z

Summary

Implement framework for generating the documentation

Part 1 of #4477

Some of the deprecated configuration options are removed with this PR.

Unit tests

Some unit tests are added

# Conflicts: # src/main/java/org/opentripplanner/standalone/config/WheelchairAccessibilityRequestMapper.java

…ed later to output doc.

This commit refactors the NodeAdaptor and add support for adding documentation. The code for building a parameter is extracted in to ParameterBuilder. There is still a few pieces left to implement, like full support for complex types and unit tests for the doc-generation. # Conflicts: # src/main/java/org/opentripplanner/standalone/config/framework/NodeAdapter.java

…ion, asZoneId, and asStringMap

codecov-commenter · 2022-09-28T17:19:49Z

Codecov Report

Base: 58.82% // Head: 59.29% // Increases project coverage by +0.47% 🎉

Coverage data is based on head (97cf17e) compared to base (74b68cf).
Patch coverage: 74.32% of modified lines in pull request are covered.

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #4478      +/-   ##
=============================================
+ Coverage      58.82%   59.29%   +0.47%     
- Complexity     11689    11816     +127     
=============================================
  Files           1535     1542       +7     
  Lines          61356    62268     +912     
  Branches        6954     6961       +7     
=============================================
+ Hits           36090    36921     +831     
- Misses         23124    23187      +63     
- Partials        2142     2160      +18

Impacted Files	Coverage Δ
.../algorithm/raptoradapter/router/TransitRouter.java	`75.89% <ø> (ø)`
...n/java/org/opentripplanner/standalone/OTPMain.java	`0.00% <0.00%> (ø)`
...nner/standalone/config/ItineraryFiltersMapper.java	`2.94% <0.00%> (-1.94%)`	⬇️
...ntripplanner/standalone/config/S3BucketConfig.java	`7.40% <0.00%> (-5.93%)`	⬇️
...ripplanner/standalone/config/VectorTileConfig.java	`10.71% <0.00%> (-4.29%)`	⬇️
...tandalone/config/feed/DemExtractConfigBuilder.java	`0.00% <0.00%> (ø)`
...nner/standalone/config/feed/DemExtractsConfig.java	`100.00% <ø> (ø)`
...standalone/config/feed/NetexFeedConfigBuilder.java	`27.02% <0.00%> (-7.46%)`	⬇️
...nner/standalone/config/feed/OsmExtractsConfig.java	`100.00% <ø> (ø)`
...nner/standalone/config/feed/TransitFeedConfig.java	`46.15% <0.00%> (-28.85%)`	⬇️
... and 68 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

leonardehrenfried · 2022-10-03T19:48:04Z

This looks generally very nice! I ran the code and it produced lines like these:

| [netexDefaults](#null)            | `object`   | *Optional*                                       | na    | TODO     |

Presumably the #null is a mistake, isn't it?

optionsome · 2022-10-07T07:23:53Z

Also there were some warnings about unused things in otp-config.json like 23:42:06.006 WARN (NodeAdapter.java:176) Unexpected config parameter: 'otpFeatures.SandboxAPIMapboxVectorTilesApi:true' in 'otp-config.json'

This issue is still there

leonardehrenfried · 2022-10-07T07:54:49Z

Yes, can confirm. The OTPFeatures are not checked correctly:

09:54:19.801 WARN (NodeAdapter.java:164) Unexpected config parameter: 'otpFeatures.FlexRouting:true' in 'otp-config.json'
09:54:19.801 WARN (NodeAdapter.java:164) Unexpected config parameter: 'otpFeatures.OptimizeTransfers:false' in 'otp-config.json'
09:54:19.801 WARN (NodeAdapter.java:164) Unexpected config parameter: 'otpFeatures.ParallelRouting:true' in 'otp-config.json'
09:54:19.801 WARN (NodeAdapter.java:164) Unexpected config parameter: 'otpFeatures.ReportApi:true' in 'otp-config.json'
09:54:19.801 WARN (NodeAdapter.java:164) Unexpected config parameter: 'otpFeatures.SandboxAPILegacyGraphQLApi:true' in 'otp-config.json'
09:54:19.801 WARN (NodeAdapter.java:164) Unexpected config parameter: 'otpFeatures.VehicleToStopHeuristics:true' in 'otp-config.json'

leonardehrenfried

Please fix the warnings about OTPFetures.

…json/ParameterBuilder.java Co-authored-by: Leonard Ehrenfried <mail@leonard.io>

t2gran · 2022-10-10T10:32:05Z

I made a temporary fix to the EnumMap causing the warning:

09:54:19.801 WARN (NodeAdapter.java:164) Unexpected config parameter: 'otpFeatures.FlexRouting:true' in 'otp-config.json'

I want to make a better fix to this - but that requires a bit of refactoring, so I would like to do that in a separate PR. I updated the unit-test to cover this case and others (witch was ok, but did not have coverage).

leonardehrenfried

I must change my review, because I just noticed a problem with the updater params.

When I use one of my previous configurations I see the following exception on startup:

12:14:44.133 INFO (ConfigLoader.java:221) Summarizing 'septa/router-config.json': {
  "updaters" : [ {
    "type" : "vehicle-positions",
    "sourceType" : "gtfs-http",
    "url" : "https://www3.septa.org/gtfsrt/septarail-pa-us/Vehicle/rtVehiclePosition.pb",
    "feedId" : "septa-rail",
    "frequencySec" : 60
  }, {
    "type" : "vehicle-rental",
    "sourceType" : "gbfs",
    "frequencySec" : 60,
    "url" : "https://gbfs.bcycle.com/bcycle_indego/gbfs.json"
  } ]
}
12:14:44.150 ERROR (OTPMain.java:55) The parameter value 'gtfs-http' is not legal. Expected one of [GBFS, SMOOVE, VILKKU, KML, PARK_API, BICYCLE_PARK_API, HSL_PARK, GTFS_RT_HTTP, GTFS_RT_FILE, GTFS_RT_VEHICLE_POSITIONS]. Parameter: updaters.[1].sourceType. Source: router-config.json.
org.opentripplanner.util.OtpAppException: The parameter value 'gtfs-http' is not legal. Expected one of [GBFS, SMOOVE, VILKKU, KML, PARK_API, BICYCLE_PARK_API, HSL_PARK, GTFS_RT_HTTP, GTFS_RT_FILE, GTFS_RT_VEHICLE_POSITIONS]. Parameter: updaters.[1].sourceType. Source: router-config.json.
	at org.opentripplanner.standalone.config.framework.json.NodeAdapter.createException(NodeAdapter.java:253)
	at org.opentripplanner.standalone.config.framework.json.ParameterBuilder.error(ParameterBuilder.java:580)
	at org.opentripplanner.standalone.config.framework.json.ParameterBuilder.lambda$parseEnum$15(ParameterBuilder.java:486)
	at java.base/java.util.Optional.orElseThrow(Optional.java:403)
	at org.opentripplanner.standalone.config.framework.json.ParameterBuilder.parseEnum(ParameterBuilder.java:485)
	at org.opentripplanner.standalone.config.framework.json.ParameterBuilder.asEnum(ParameterBuilder.java:172)
	at org.opentripplanner.standalone.config.updaters.VehiclePositionsUpdaterConfig.create(VehiclePositionsUpdaterConfig.java:24)
	at org.opentripplanner.standalone.config.UpdatersConfig.<init>(UpdatersConfig.java:146)
	at org.opentripplanner.standalone.config.RouterConfig.<init>(RouterConfig.java:94)
	at org.opentripplanner.standalone.config.ConfigLoader.loadRouterConfig(ConfigLoader.java:148)
	at org.opentripplanner.standalone.config.ConfigModel.<init>(ConfigModel.java:64)
	at org.opentripplanner.standalone.config.configure.LoadConfigModule.providesModel(LoadConfigModule.java:39)
	at org.opentripplanner.standalone.config.configure.LoadConfigModule_ProvidesModelFactory.providesModel(LoadConfigModule_ProvidesModelFactory.java:42)
	at org.opentripplanner.standalone.config.configure.LoadConfigModule_ProvidesModelFactory.get(LoadConfigModule_ProvidesModelFactory.java:33)
	at org.opentripplanner.standalone.config.configure.LoadConfigModule_ProvidesModelFactory.get(LoadConfigModule_ProvidesModelFactory.java:13)
	at dagger.internal.DoubleCheck.get(DoubleCheck.java:47)
	at org.opentripplanner.standalone.configure.DaggerLoadApplicationFactory$LoadApplicationFactoryImpl.configModel(DaggerLoadApplicationFactory.java:129)
	at org.opentripplanner.standalone.configure.LoadApplication.config(LoadApplication.java:65)
	at org.opentripplanner.standalone.OTPMain.startOTPServer(OTPMain.java:112)
	at org.opentripplanner.standalone.OTPMain.main(OTPMain.java:53)
12:14:44.151 INFO (OtpStartupInfo.java:43) OTP SHUTTING DOWN (version: 2.2.0-SNAPSHOT, ser.ver.id: 67, commit: ccfa4b706296c0858aa87f9877d2d97ed2e5c64c, branch: ccfa4b706296c0858aa87f9877d2d97ed2e5c64c)

I've written a unit test which allows you to easily reproduce the problem: leonardehrenfried@781fd9a

leonardehrenfried · 2022-10-11T10:24:16Z

Or is the change of the enum values intentional?

# Conflicts: # src/main/java/org/opentripplanner/standalone/config/updaters/GtfsRealtimeAlertsUpdaterConfig.java # src/main/java/org/opentripplanner/standalone/config/updaters/MqttGtfsRealtimeUpdaterConfig.java # src/main/java/org/opentripplanner/standalone/config/updaters/WFSNotePollingGraphUpdaterConfig.java # src/main/java/org/opentripplanner/standalone/config/updaters/WebsocketGtfsRealtimeUpdaterConfig.java

t2gran · 2022-10-13T10:47:35Z

For the updaters we used to map a set of string type and data-sources into enum types used inside OTP. This is flexible and make it easier to change the internal model, but it also can be very confusing and might not give us that much - it is relatively easy to update the config files. So, after talking to @leonardehrenfried I changed the source types "gtfs-http" to "gtfs-rt-http" and "gtfs-file" to "gtfs-rt-file". The error message you will get if you do not update the config is this:

org.opentripplanner.util.OtpAppException: The parameter value 'gtfs-http' is not legal. Expected one of [GBFS, SMOOVE, VILKKU, KML, PARK_API, BICYCLE_PARK_API, HSL_PARK, GTFS_RT_HTTP, GTFS_RT_FILE, GTFS_RT_VEHICLE_POSITIONS]. Parameter: updaters.[1].sourceType. Source: RouterConfigTest.
        :
	at org.opentripplanner.standalone.config.framework.json.ParameterBuilder.asEnum(ParameterBuilder.java:172)
	at org.opentripplanner.standalone.config.updaters.VehiclePositionsUpdaterConfig.create(VehiclePositionsUpdaterConfig.java:16)

We can further improve the error message later if we list all sourceTypes for each updater type.

…rification

leonardehrenfried

I re-ran the IBI smoke test suite with this PR and all tests pass.

t2gran added 19 commits September 28, 2022 16:26

refactor: Move NodeAdopter into config.framework.

11a4537

# Conflicts: # src/main/java/org/opentripplanner/standalone/config/WheelchairAccessibilityRequestMapper.java

refactor: Make NodeAdaptor part of root config objects - this is need…

f6d4e35

…ed later to output doc.

refactor: Add the ability to format a Markdown table with TableFormatter

a9eacf8

feature: Generate BuildConfig.json documentation.

23a77ed

refactor: move NodeAdaptor into framework.json

613e79a

refactor: Insert TODOs + small cleanup

89cc2c1

refactor: Inline NodeAdapter#ofWDoc

6c045f4

refactor: Inline NodeAdapter#asBoolean

f9da794

refactor: Inline NodeAdapter#asDouble...

74ed0e6

refactor: Inline NodeAdapter#asInt and asLong

6dd7c27

refactor: Inline NodeAdapter#asText methods

2db5729

refactor: Inline NodeAdapter#asEnum methods

eed00d4

refactor: Inline NodeAdapter#asFeedScopedId methods

678f435

refactor: Inline NodeAdapter#asLocale

4867311

refactor: Inline NodeAdapter#asDateOrRelativePeriod

3b4872c

refactor: Inline NodeAdapter#asDuration methods

70734a2

refactor: Inline NodeAdapter: asPattern, asUri, asUris, asLinearFunct…

720a058

…ion, asZoneId, and asStringMap

refactor: Cleanup NodeAdapterTest

2febbf0

t2gran added release management !Documentation Improve documentation targeting users and/or developers. !Technical Debt Improve code quality, no functional changes. +Config Change This PR might require the configuration to be updated. labels Sep 28, 2022

t2gran added this to the 2.2 milestone Sep 28, 2022

t2gran requested a review from a team as a code owner September 28, 2022 17:12

t2gran mentioned this pull request Sep 28, 2022

Generate Configuration Documentation #4477

Closed

32 of 51 tasks

hannesj requested review from leonardehrenfried and optionsome September 29, 2022 13:44

leonardehrenfried reviewed Oct 7, 2022

View reviewed changes

Comment thread src/main/java/org/opentripplanner/standalone/config/framework/json/ParameterBuilder.java Outdated

leonardehrenfried reviewed Oct 7, 2022

View reviewed changes

Comment thread src/main/java/org/opentripplanner/standalone/config/framework/json/ParameterBuilder.java Outdated

leonardehrenfried requested changes Oct 7, 2022

View reviewed changes

t2gran and others added 2 commits October 10, 2022 10:32

Update src/main/java/org/opentripplanner/standalone/config/framework/…

66e6196

…json/ParameterBuilder.java Co-authored-by: Leonard Ehrenfried <mail@leonard.io>

fix: TimeZone error in test

a5ef077

t2gran force-pushed the otp2_print_config_doc branch from c033824 to a5ef077 Compare October 10, 2022 08:34

t2gran added 2 commits October 10, 2022 12:24

refactor: Remove doc-config-calls from NodeAdapterTest

cd7333d

review: Fix so, EnumMap does not print unused parameter warnings.

ccfa4b7

leonardehrenfried approved these changes Oct 10, 2022

View reviewed changes

hannesj requested review from abyrd and optionsome October 11, 2022 08:17

optionsome previously approved these changes Oct 11, 2022

View reviewed changes

Comment thread src/main/java/org/opentripplanner/standalone/config/framework/json/NodeInfo.java Outdated

leonardehrenfried requested changes Oct 11, 2022

View reviewed changes

review: Fix parse error in updaters

a0f0fe6

t2gran dismissed optionsome’s stale review via a0f0fe6 October 13, 2022 10:26

t2gran added 2 commits October 13, 2022 13:39

refactor: Copy build-config from doc and use it in a unit-test for ve…

97cf17e

…rification

refactor: Use builder for "skipChild" NodeInfo

1df96ac

leonardehrenfried approved these changes Oct 13, 2022

View reviewed changes

t2gran added the +Bump Serialization Id Add this label if you want the serialization id automatically bumped after merging the PR label Oct 13, 2022

optionsome approved these changes Oct 13, 2022

View reviewed changes

t2gran merged commit a136941 into opentripplanner:dev-2.x Oct 13, 2022

t2gran pushed a commit that referenced this pull request Oct 13, 2022

Add changelog entry for #4478 [ci skip]

1f09eb6

t2gran pushed a commit that referenced this pull request Oct 13, 2022

Bump serialization version id for #4478

42061d4

t2gran deleted the otp2_print_config_doc branch October 13, 2022 13:54

Navigation Menu

Repository navigation

Generate Configuration Documentation - Part 1 - #4478

Generate Configuration Documentation - Part 1#4478
t2gran merged 36 commits into
opentripplanner:dev-2.xfrom
entur:otp2_print_config_doc

t2gran commented Sep 28, 2022

Uh oh!

codecov-commenter commented Sep 28, 2022 •

edited

Loading

Uh oh!

leonardehrenfried commented Oct 3, 2022

Uh oh!

optionsome commented Oct 7, 2022

Uh oh!

leonardehrenfried commented Oct 7, 2022

Uh oh!

Uh oh!

Uh oh!

leonardehrenfried left a comment

Uh oh!

t2gran commented Oct 10, 2022 •

edited

Loading

Uh oh!

Uh oh!

leonardehrenfried left a comment •

edited

Loading

Uh oh!

leonardehrenfried commented Oct 11, 2022

Uh oh!

t2gran commented Oct 13, 2022

Uh oh!

leonardehrenfried left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

t2gran commented Sep 28, 2022

Summary

Unit tests

Uh oh!

codecov-commenter commented Sep 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

leonardehrenfried commented Oct 3, 2022

Uh oh!

optionsome commented Oct 7, 2022

Uh oh!

leonardehrenfried commented Oct 7, 2022

Uh oh!

Uh oh!

Uh oh!

leonardehrenfried left a comment

Choose a reason for hiding this comment

Uh oh!

t2gran commented Oct 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

leonardehrenfried left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leonardehrenfried commented Oct 11, 2022

Uh oh!

t2gran commented Oct 13, 2022

Uh oh!

leonardehrenfried left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Sep 28, 2022 •

edited

Loading

t2gran commented Oct 10, 2022 •

edited

Loading

leonardehrenfried left a comment •

edited

Loading