Modify

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#10867 closed defect (fixed)

notes are download with a limit of 1000 without telling the user about reaching this limit

Reported by: aseerel4c26 Owned by: ToeBee
Priority: minor Milestone: 15.04
Component: Core notes Version: latest
Keywords: Cc:

Description

apparently (judging from my terminal log which shows an GET parameter of "limit=1000") notes are download with a limit of 1000 without telling the user about this (and the user also did not set this limit). If the API then returns 1000 hits it is very likely that some notes from the selected area are missing (because there will be more than 1000). The user will not know about this (no error message, note even in the terminal) and would think that he got all notes in this area.

Suggestion (and easy fix): If a limit is needed, then the user should be told (in such a unobstrusive popup bubble thingy at the bottom left corner) about reaching the limit (1000 downloaded notes). Something like this: your notes API request was limited to 1000 notes. There seem to be more notes than this in your area. You may want to choose a smaller area.

JOSM Version 7819.

Attachments (0)

Change History (17)

comment:1 Changed 5 years ago by ToeBee

The OSM API defaults to 100 if no limit is given and has a hard upper limit of 10,000. (It was actually 9,999 in the code and 10,000 in the documentation until I discovered this discrepancy while writing the notes plugin and submitted a patch to the rails port :)

I limited JOSM to request 1,000 by default although this value is based on the osm.notes.downloadLimit preference so you can set it higher which is what I would suggest doing for right now. Eventually this should be configurable in a UI somewhere instead of having to dig in the advanced preferences list. And maybe I should just default to 10,000? Not sure on that.

But either way, your suggestion of popping up a warning if the download limit is reached is probably a good idea.

comment:2 Changed 5 years ago by aseerel4c26

A number limit reached warning would be good in any way, I think. Maybe, in addition to the advice of a smaller area, the warning popup could offer to increase the limit or simply provide a link to the docu (where the advanced pref is mentioned).

Regarding the default limit: I do not know. What is the danger of 10000? First, the user needs to select an area which is really that big to contain that many notes. Then: The user gets an out of RAM? Not sure how much notes need. But since the API defaults to 100 I think we should not default to the max. 1000 may be fine until we know better.

comment:3 Changed 5 years ago by Don-vip

Component: CoreCore notes

Moving notes tickets to their own component

comment:4 Changed 5 years ago by Don-vip

Keywords: notes removed

comment:5 Changed 4 years ago by simon04

Resolution: fixed
Status: newclosed

In 8216/josm:

fix #10867 - Notes download: inform user about reaching download limit

comment:6 Changed 4 years ago by simon04

Milestone: 15.04

comment:7 Changed 4 years ago by simon04

In 8217/josm:

fix #10867 - Notes download: inform user about reaching download limit (2/2)

comment:8 Changed 4 years ago by Klumbumbus

Was the osm api hard limit changed from 10k to 50k? edit: ok, answered myself. now I see, you changed the nodes download limit and not the notes download limit.

Last edited 4 years ago by Klumbumbus (previous) (diff)

comment:9 Changed 4 years ago by simon04

In 8230/josm:

see #10867 - Note limit is still 10000

comment:10 in reply to:  8 Changed 4 years ago by simon04

Replying to Klumbumbus:

Was the osm api hard limit changed from 10k to 50k? edit: ok, answered myself. now I see, you changed the nodes download limit and not the notes download limit.

I confused node with note while looking for the constant in the source … I set it back to the correct 10k in r8230. Thank you!

Anyway, the API would have told when loading 30k notes … ;-)

comment:11 Changed 4 years ago by aseerel4c26

The warning if the download limit is reached (tested with 500 and 1000) seems to work fine, thanks simon04! :-)

comment:12 Changed 4 years ago by stoecker

Mustn't r8216 limit change also be reversed?

comment:13 Changed 4 years ago by simon04

This has been done in r8230?
10k is correct according to the source code of openstreetmap-website …

comment:14 in reply to:  13 Changed 4 years ago by stoecker

Replying to simon04:

This has been done in r8230?

Don't think so:
BoundingBoxDownloader.java line 214

comment:15 Changed 4 years ago by Klumbumbus

There is no line 214!?

comment:16 Changed 4 years ago by stoecker

Ah. I overlooked r8218.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain ToeBee.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.