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

If Nodes / Spans GeoJSOn uploaded and conversion to JSON crashes, show nice messages to user #84

Open
odscjames opened this issue Feb 7, 2023 · 5 comments

Comments

@odscjames
Copy link
Collaborator

Think this is what happened with Open-Telecoms-Data/lib-cove-ofds#69

We should fix in library but also in Cove add some crash protection

@duncandewhurst
Copy link
Contributor

I think the ultimate solution to this might be:

@odscjames
Copy link
Collaborator Author

The only way I see a GeoJSON Schema removing the need for this work is if we validate GeoJSON before conversion and refuse to process if not perfect. That doesn't seem a great option. There could still be errors that we can't protect against in JSONSchema and it seems generally it would be better to try and be forgiving where possible. (A GeoJSON schema may be neat for other reasons but I don't think it helps here)

@odscjames
Copy link
Collaborator Author

Screenshot from 2023-02-17 11-44-02

This is the current screen that comes up - I think this might already be done actually?

odscjames added a commit that referenced this issue Feb 17, 2023
Still to fix:
Invalid template name in 'extends' tag: ''. Got this from the 'request.current_app_base_template' variable.

#84
@duncandewhurst
Copy link
Contributor

That's better than showing the Python error but it won't help users troubleshoot problems with the data that they submitted like the one in Open-Telecoms-Data/lib-cove-ofds#69 (comment)

@odscjames
Copy link
Collaborator Author

Yes - the first part of that work is making the library more robust so it defensively checks data and doesn't crash on bad data. We're doing that in issues like the one you linked. (Fix already in lib but not live yet).

Then it's seeing how the conversion can raise nice messages to the users. The GeoJSON -> JSON library already reports on problems like inconsistent_network_ids, so it's putting more checks in there and raising those messages to the users. Maybe a schema for GeoJSON can help with that? (tho that doesn't remove the need for other work)

Are you thinking there is other things we could do here? Maybe a chat to go over ideas would be best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants