Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

wrong value comments_count reported by API #401

Closed
GerdP opened this issue Apr 25, 2024 · 8 comments · Fixed by #402
Closed

wrong value comments_count reported by API #401

GerdP opened this issue Apr 25, 2024 · 8 comments · Fixed by #402
Labels

Comments

@GerdP
Copy link

GerdP commented Apr 25, 2024

The response from API https://api.openstreetmap.org/api/0.6/changeset/79214306
says comments_count="0" while https://api.openstreetmap.org/api/0.6/changesets?changesets=79214306
correctly says comments_count="1"

See also https://josm.openstreetmap.de/ticket/23642#comment:2 which shows the responses.

@Woazboat
Copy link
Contributor

Woazboat commented Apr 25, 2024 via email

@GerdP
Copy link
Author

GerdP commented Apr 25, 2024

Probably to avoid caching unwanted data. The call is only executed to fill the list of changesets. Do you suggest to change JOSM code instead of fixing the wrong API result?

@mmd-osm
Copy link
Collaborator

mmd-osm commented Apr 25, 2024

Thank you for reporting this issue. Indeed, this looks like a regression in 0.9.1. comments_count should be 1 in both cases.

By the way, changesets?changesets=79214306 is implemented on Rails only, that's why the changes in #388 have no impact on this endpoint.

Link to JOSM discussion: https://josm.openstreetmap.de/ticket/23642

@mmd-osm mmd-osm added the bug label Apr 25, 2024
@mmd-osm
Copy link
Collaborator

mmd-osm commented Apr 25, 2024

@tomhughes : one option I see here is to route the /changeset endpoint to Rails until we have fixed the bug on CGImap side: https://github.com/openstreetmap/chef/blob/16da621fbfec07c850ffc210e300556db3f8d4b2/cookbooks/dev/templates/default/apache.rails.erb#L46

This depends a bit how severe the issue is for JOSM. It has been created with "normal" priority, so we might as well wait until we have the fix available.

FIx should be available on the dev instance in the next 30 mins. Example: https://master.apis.dev.openstreetmap.org/api/0.6/changeset/299208 vs. https://master.apis.dev.openstreetmap.org/api/0.6/changeset/299208?include_discussion

New release 0.9.2 based on 0.9.1 + cherry picking c9c222c

@mmd-osm
Copy link
Collaborator

mmd-osm commented Apr 25, 2024

New build is in progress: https://launchpad.net/~mmd-osm/+archive/ubuntu/cgimap-902-jammy
Operations issue is linked below. Closing here.

@Woazboat
Copy link
Contributor

Woazboat commented Apr 25, 2024

Probably to avoid caching unwanted data. The call is only executed to fill the list of changesets. Do you suggest to change JOSM code instead of fixing the wrong API result?

No? Obviously the API should be fixed. I assumed the third request for the single changeset details as listed in the josm issue is triggered by clicking on the button to show the changeset comments. That's why it seemed strange that it didn't set the include_discussion parameter, considering fetching the comments was the main purpose.

https://josm.openstreetmap.de/ticket/23642#comment:1

2024-04-25 09:00:38.390 INFO: GET https://api.openstreetmap.org/api/0.6/way/37407704/history -> HTTP/1.1 200 (175 ms)
2024-04-25 09:00:38.541 INFO: GET https://api.openstreetmap.org/api/0.6/changesets?changesets=1770190,16412412,79214306 -> HTTP/1.1 200 (136 ms; 636 B)
2024-04-25 09:00:40.660 INFO: GET https://api.openstreetmap.org/api/0.6/changeset/79214306 -> HTTP/1.1 200 (38 ms)

I haven't actually been able to reproduce the josm issue. I always get two GET requests for the changeset, one without the parameter and then one with the parameter right after.

2024-04-25 23:56:01.446 INFO: GET https://api.openstreetmap.org/api/0.6/way/37407704/history -> HTTP_2 200 (36 ms)
2024-04-25 23:56:01.515 INFO: GET https://api.openstreetmap.org/api/0.6/changesets?changesets=1770190,16412412,79214306 -> HTTP_2 200 (62 ms; 636 B)
2024-04-25 23:56:05.335 INFO: GET https://api.openstreetmap.org/api/0.6/changeset/79214306 -> HTTP_2 200 (43 ms)
2024-04-25 23:56:05.482 INFO: GET https://api.openstreetmap.org/api/0.6/changeset/79214306?include_discussion=true -> HTTP_2 200 (32 ms)

Edit: Fix is already deployed, so that's why.

@GerdP
Copy link
Author

GerdP commented Apr 26, 2024

Yes, I also wondered why JOSM sends two requests, The patch 23642-2.patch which I attached to the JOSM ticket solves this:

2024-04-26 06:55:50.890 INFO: GET https://api.openstreetmap.org/api/0.6/way/37407704/history -> HTTP/1.1 200 (106 ms)
2024-04-26 06:55:51.034 INFO: GET https://api.openstreetmap.org/api/0.6/changesets?changesets=1770190,16412412,79214306 -> HTTP/1.1 200 (132 ms; 636 B)
2024-04-26 06:55:53.141 INFO: GET https://api.openstreetmap.org/api/0.6/changeset/79214306?include_discussion=true -> HTTP/1.1 200 (37 ms)

@GerdP
Copy link
Author

GerdP commented Apr 26, 2024

Never used it before but there is a side window "Changesets" which shows the changesets for the downloaded data. For this scenario 23642-2.patch would possibly download more data than before.

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

Successfully merging a pull request may close this issue.

3 participants