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

Syncs systems.csv with transport.data.gouv #593

Merged
merged 4 commits into from
Jan 25, 2024
Merged

Syncs systems.csv with transport.data.gouv #593

merged 4 commits into from
Jan 25, 2024

Conversation

richfab
Copy link
Contributor

@richfab richfab commented Jan 22, 2024

This PR syncs systems.csv with GBFS data sources from transport.data.gouv.fr.

  • Removed Bird Marseille (ceased operations)
  • Added levélo Marseille
  • Updated LoVélo Rouen (endpoint was system_information instead of gbfs)
  • Removed Vélivert Saint Étienne duplicate and updated website URL
  • Added Vélo'Baie,Saint-Brieuc

isabelle-dr
isabelle-dr reviewed Jan 23, 2024
View reviewed changes
Copy link
Contributor

@isabelle-dr isabelle-dr left a comment

Choose a reason for hiding this comment

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

Two of these feeds need a token, should we remove the key from the URL and provide it in the last column?

systems.csv Outdated
@@ -345,13 +344,14 @@ FR,Dott Paris,Paris,dott-paris,https://ridedott.com,https://gbfs.api.ridedott.co
FR,Dott Tignes,Tignes,dott-tignes,https://ridedott.com,https://gbfs.api.ridedott.com/public/v2/tignes/gbfs.json,
FR,Dott Val-d’isere,Val-d’isere,dott-val-d’iser,https://ridedott.com/,https://gbfs.api.ridedott.com/public/v2/val-d%E2%80%99isere/gbfs.json,
FR,LE vélo STAR,Rennes,le_velo_star,https://www.star.fr/le-velo/nos-offres/vls/,https://eu.ftp.opendatasoft.com/star/gbfs/gbfs.json,
FR,Levélo,Marseille,levelo_inurba_marseille,https://levelo.ampmetropole.fr,https://api.omega.fifteen.eu/gbfs/2.2/marseille/en/gbfs.json?&key=MjE0ZDNmMGEtNGFkZS00M2FlLWFmMWItZGNhOTZhMWQyYzM2,
Copy link
Contributor

Choose a reason for hiding this comment

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

This format with a key would be a first in systems.csv I believe

Copy link
Contributor

Choose a reason for hiding this comment

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

According to the files listed, the auto-discovery URL is https://api.omega.fifteen.eu/gbfs/2.2/marseille/en/gbfs.json and users need a token to access the data. Should we include the token in the last column instead?

Copy link
Contributor Author

@richfab richfab Jan 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Currently the column Authentication Info URL is to provide a URL to a human-readable page describing how the authentication should be performed (README.md).

I could not find a page explaining how to retrieve the token. This public token is directly appended to the URL on transport.data.gouv and on ampmetropole.opendatasoft.com.

I suggest to rename the column Authentication Info URL to Authentication Info, update the description in the README.md and move the token from the URL to the Authentication Info column.

Copy link
Contributor Author

@richfab richfab Jan 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I added commits to this PR to make the change.

systems.csv Outdated
FR,VélO2,Cergy-Pontoise,cergy-pontoise,http://www.velo2.cergypontoise.fr/,https://transport.data.gouv.fr/gbfs/cergy-pontoise/gbfs.json,
FR,Vélo'Baie,Saint-Brieuc,saintbrieuc,https://www.saintbrieuc-armor-agglo.bzh/velobaie,https://gateway.prod.partners-fs37hd8.zoov.site/gbfs/2.2/saintbrieuc/en/gbfs.json?key=YmE1ZDVlNDYtMGIwNy00MGEyLWIxZWYtNGEwOGQ4NTYxNTYz,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about the tokens

@richfab richfab requested a review from isabelle-dr January 24, 2024 09:36
mplsmitch
mplsmitch reviewed Jan 24, 2024
View reviewed changes
README.md
@@ -126,7 +126,7 @@ Location | Yes| Primary city in which the system is located, followed by the 2-l
System ID | Yes | ID for the system. This MUST match the `system_id` field in `system_information.json`.
URL | Yes | URL for the system from the `url` field in `system_information.json`. If the `url` field is not included in `system_information.json` this SHOULD be the primary URL for the system operator.
Auto-Discovery URL | Yes | URL for the system's `gbfs.json` auto-discovery file.
Authentication Info | Conditionally REQUIRED | If authentication is required, this MUST contain a URL to a human-readable page describing how the authentication should be performed and how credentials can be created, or directly contain the public key-value pair to be added to the URL.
Authentication Info | Conditionally REQUIRED | If authentication is required, this MUST contain a URL to a human-readable page describing how the authentication should be performed and how credentials can be created, or directly contain the public key-value pair to append to the feed URLs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This wouldn't be a key-value pair. I think what you want to say here is "contain the API key or token to append to the feed URLs"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. How can we make explicit that the name of the parameter is required too?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is getting overly complicated - I don't see any reason to separate the key from the URL in the auto-discovery column. What's the value in that? We've got lots of URLS with parameters for example: https://qickscooters.rideatom.com/gbfs/v2_2/en/gbfs?id=104 the only difference is that in this case it's an API key not a system_id but they function in the same way since the API key doesn't change. It's my understanding that the last column was added to support Auth schemes that use tokens or other forms of Auth that are subject to change and therefore needed a URL to provide additional info on how to get access.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem here is that gbfs.json references the sub-feeds without the key. It needs to be appended manually to them. So I suggest keeping the "key value pair" language.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm sorry - I misunderstood the problem. Ortwin is correct. I didn't realize that the URLS in gbfs.json lacked the key. However I would also argue that this is broken because feeds[].url in gbfs.json should contain a valid URL pointing to the file. Maybe we should consider updating that definition and requiring valid URLs. I've seen this lots of times the past and I just left the feeds out of systems.csv until they fixed the gbfs.json file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you both for your input!

I would also argue that this is broken because feeds[].url in gbfs.json should contain a valid URL pointing to the file.

I agree. I informed the producer that it would be best to remove the public key or append it to all URLs in gbfs.json.

Maybe we should consider updating that definition and requiring valid URLs.

+1

isabelle-dr
isabelle-dr approved these changes Jan 25, 2024
View reviewed changes
Copy link
Contributor

@isabelle-dr isabelle-dr left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the input @mplsmitch

@richfab richfab merged commit a3d68f6 into master Jan 25, 2024
1 check passed
@richfab richfab deleted the feat/sync-with-transport-data-gouv branch January 25, 2024 16:08
richfab added a commit to futuretap/gbfs that referenced this pull request May 17, 2024
@richfab
Squashed commit of the following:
db2a467 
commit e0c48b2
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Mon Apr 29 11:13:47 2024 +0200

    Clarify vehicle_id persistence rule (MobilityData#632)

    This PR:
    - Makes the rule about vehicle_id persistence more explicit: rotation is between rentals (not every time the feed is loaded)
    - Updates a forgotten reference to free_bike_status.json (now vehicle_status.json)

    Thanks @testower for flagging!

commit 73d135d
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Tue Apr 16 15:22:18 2024 +0200

    Set the migration guide as the v3.0 Release Notes (MobilityData#628)

    This PR sets the migration guide article as the v3.0 Release Notes: https://mobilitydata.org/how-to-upgrade-to-gbfs-v3-0/

commit 457cb71
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Thu Apr 11 14:19:32 2024 +0200

    Update current version to v3.0 (MobilityData#625)

    * Withdraw PR MobilityData#546 Floating Interval in price

    * Withdraw PR MobilityData#457 Reservation Price

    * Remove -RC and -RC2 from the spec

    * Update current version number in spec

    * Fix earlier version URLs

    * Update current version in README to v3.0

    * Set version history URL to Release Notes

commit f4dc269
Author: Mitch Vars <mplsmitch@gmail.com>
Date:   Thu Apr 11 01:10:58 2024 -0500

    Correction systems.csv (MobilityData#623)

    Fix missing csv comma

commit 940cd63
Author: Mitch Vars <mplsmitch@gmail.com>
Date:   Tue Apr 9 11:11:26 2024 -0500

    Update systems.csv to remove dead feeds (MobilityData#622)

    * Update systems.csv

    Remove obsolete feeds

    * Update systems.csv

    * Add 4 feeds that now return status code 200

    ---------

    Co-authored-by: Fabien Richard-Allouard <fabien@mobilitydata.org>

commit a77c5c2
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Tue Apr 9 15:40:21 2024 +0200

    Mark system_hours and system_calendar as removed (MobilityData#621)

    ## Problem
    The files system_hours.json and system_calendar.json were removed in MobilityData#328. However, the spec says these files were “deprecated”, which usually means permitted, but discouraged.

    ## Solution
    Replace "deprecated” with "removed" for the files system_hours.json and system_calendar.json in the spec.

commit 45e944a
Author: indigotachizawa <tachizawa@indigo.co.jp>
Date:   Mon Apr 8 21:01:42 2024 +0900

    add new service 'kotobike' (MobilityData#620)

commit 2f5aaad
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Wed Mar 20 17:07:56 2024 +0100

    Authentication information for Voi Brussels (MobilityData#613)

    This PR adds authentication information for Voi Brussels.

    > Access and usage of Voi’s GBFS data is subject to the following licensing terms: https://www.voi.com/voi-licencing-agreement.

    More info on the Belgium NAP: https://www.transportdata.be/dataset/voi

commit a72d597
Author: Mitch Vars <mplsmitch@gmail.com>
Date:   Wed Mar 13 12:03:36 2024 -0500

    Update Nextbike URLs (MobilityData#610)

    * Update Nextbike URLs

    * Update 4 additional Nextbike URLs

    ---------

    Co-authored-by: Fabien Richard-Allouard <fabien@mobilitydata.org>

commit 4b4d3f7
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Wed Mar 13 14:31:38 2024 +0100

    Fix FAQ URL in issue template (MobilityData#609)

    This PR fixes the FAQ URL in the issue template.

    Before | After
    -- | --
    https://gbfs.mobilitydata.org/faq | https://gbfs.org/learn/faq/

commit b286e95
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Fri Mar 1 17:06:20 2024 +0100

    Update roadmap URL in README (MobilityData#605)

    This PR updates the roadmap URL in the README:

    - Before: Github wiki
    - After: https://portal.productboard.com/26qpteg4wct9px3jts94uqv8/tabs/99-planned

commit bea68ce
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Tue Feb 27 16:03:23 2024 +0100

    Delete obselete feed Donkey Republic Barcelona (MobilityData#599)

commit b9d8390
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Wed Feb 14 12:54:14 2024 +0100

    Fix geofencing examples (MobilityData#595)

    * Remove overlapping polygon from example

    * Change rule to override global rules in example

commit 6a9150c
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Mon Jan 29 14:16:55 2024 +0100

    Fix markdown formatting (MobilityData#596)

    This PR fixes the markdown formatting when governance.md is imported on https://gbfs.org/specification/process/

commit a3d68f6
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Thu Jan 25 17:08:37 2024 +0100

    Syncs systems.csv with transport.data.gouv (MobilityData#593)

    * Syncs systems.csv with transport.data.gouv

    * Move public keys to Authentication Info column

    * Update Authentication Info description in README

    * Rephrase Authentication Info description in README

commit 0775ee4
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Tue Jan 23 17:39:47 2024 +0100

    Label global_rules field as added in v3.0-RC (MobilityData#594)

    This editorial change adds a missing label under the field `global_rules` to explain that the field was added in v3.0-RC.

commit e9fc5aa
Author: Ortwin Gentz, FutureTap <futuretap@users.noreply.github.com>
Date:   Mon Jan 22 15:39:26 2024 +0100

    gbfs.md: Fix language about iOS links (MobilityData#585)

commit 411b4f4
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Mon Jan 15 15:44:59 2024 +0100

    README.md: Add link to v2.3 Release Notes (MobilityData#592)

    This PR adds the link to the [v2.3 Release Notes](https://github.com/MobilityData/gbfs/releases/tag/v2.3) in the Current Version table of the [README.md](https://github.com/MobilityData/gbfs/blob/master/README.md).

commit 32ccae4
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Mon Jan 15 15:00:59 2024 +0100

    GBFS Governance Revisions (MobilityData#579)

    * Add release cycles to the governance

    * Link README to governance.md for Release Cycles

    * Simplify MINOR version release line

    * Update stale bot

    * Typo

commit 4e9c72f
Author: Merja Kajava <50193837+merjakaj@users.noreply.github.com>
Date:   Thu Jan 11 17:49:29 2024 +0200

    Updated location names in systems.csv (MobilityData#590)

commit 43b9be7
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Thu Jan 11 14:39:23 2024 +0100

    Add country name when location is empty (MobilityData#589)

commit 3806cdc
Author: Merja Kajava <50193837+merjakaj@users.noreply.github.com>
Date:   Thu Jan 11 12:37:46 2024 +0200

    Removed nextbike_fo from systems.csv (MobilityData#588)

commit f6a9c56
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Wed Jan 10 16:54:59 2024 +0100

    Location formatting recommendation in systems.csv (MobilityData#587)

    * Location formatting recommendation in systems.csv

    This PR adds a recommendation in README.md about the formatting of the system location in systems.csv.

    * Remove country code from formatting recommendation

    * Remove country code from Location column

commit a36a103
Author: Fabien Richard-Allouard <fabien@mobilitydata.org>
Date:   Wed Jan 10 14:52:57 2024 +0100

    Update VéloZef (Brest, FR) system_id (MobilityData#586)

    This PR updates the system_id of VéloZef (Brest, FR) to match the system_id from the [feed](https://api.prod.partners-fs37hd8.zoov.eu/gbfs/2.2/brest/en/system_information.json?&key=OGNhZDNjMDQtYTA0Yi00NzU2LWE0MTItOGJlYzE1Y2E4NGEx): `velozef`.

    Thanks to @XioNoX for noticing ❤️
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mplsmitch mplsmitch mplsmitch left review comments

@futuretap futuretap futuretap left review comments

@isabelle-dr isabelle-dr isabelle-dr approved these changes

@davidgamez davidgamez Awaiting requested review from davidgamez

Assignees
No one assigned
Labels
systems.csv
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@richfab @futuretap @mplsmitch @isabelle-dr