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.

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

Remove parsing of GBFS timezones #4313

Merged

Conversation

leonardehrenfried
Copy link
Member

Summary

Sadly there are many invalid GBFS feeds out there which send an incorrect timezone.

Examples:

Unfortunately, the validator doesn't validate this yet: MobilityData/gbfs-validator#76

This causes OTP to completely reject the feed.

Since we actually don't really need the timezone I've skipped parsing it and using an Instant instead.

@leonardehrenfried leonardehrenfried added IBI Developed by or important for IBI Group Skip Changelog labels Jul 27, 2022
@leonardehrenfried leonardehrenfried requested a review from a team as a code owner July 27, 2022 09:52
@leonardehrenfried
Copy link
Member Author

cc @demory @miles-grant-ibigroup

hannesj
hannesj previously approved these changes Jul 27, 2022
@@ -18,7 +16,6 @@ public class VehicleRentalSystem {
public final String phoneNumber;
public final String email;
public final String feedContactEmail;
public final ZoneId timezone;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be ok to just store it as String here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, why not.

@codecov-commenter
Copy link

Codecov Report

Merging #4313 (676692f) into dev-2.x (d90ea9a) will increase coverage by 0.05%.
The diff coverage is 66.66%.

@@              Coverage Diff              @@
##             dev-2.x    #4313      +/-   ##
=============================================
+ Coverage      57.13%   57.18%   +0.05%     
- Complexity     10779    10786       +7     
=============================================
  Files           1429     1428       -1     
  Lines          58086    58020      -66     
  Branches        6728     6728              
=============================================
- Hits           33185    33181       -4     
+ Misses         22891    22830      -61     
+ Partials        2010     2009       -1     
Impacted Files Coverage Δ
...r/routing/vehicle_rental/VehicleRentalStation.java 90.38% <ø> (ø)
...er/routing/vehicle_rental/VehicleRentalSystem.java 100.00% <ø> (ø)
...r/routing/vehicle_rental/VehicleRentalVehicle.java 33.33% <ø> (ø)
...ental/datasources/GbfsFreeVehicleStatusMapper.java 0.00% <0.00%> (ø)
...le_rental/datasources/GbfsStationStatusMapper.java 58.69% <100.00%> (-0.88%) ⬇️
...ental/datasources/GbfsSystemInformationMapper.java 65.38% <100.00%> (ø)
...anner/standalone/configure/OTPAppConstruction.java 11.76% <0.00%> (-2.99%) ⬇️
...org/opentripplanner/transit/service/StopModel.java 59.13% <0.00%> (-2.16%) ⬇️
...pentripplanner/transit/service/StopModelIndex.java 87.50% <0.00%> (-1.79%) ⬇️
...es/layers/stops/DigitransitStopPropertyMapper.java 75.75% <0.00%> (-1.39%) ⬇️
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d90ea9a...676692f. Read the comment docs.

@leonardehrenfried leonardehrenfried merged commit 635eb4d into opentripplanner:dev-2.x Jul 28, 2022
@leonardehrenfried leonardehrenfried deleted the remove-gbfs-timezone branch August 4, 2022 16:57
@t2gran t2gran added this to the 2.2 milestone Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IBI Developed by or important for IBI Group Skip Changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants