mate-desktop / libmateweather

Library to access weather information from online services
https://mate-desktop.org
GNU Lesser General Public License v2.1
21 stars 24 forks source link

Fix metar response: Invalid field name(s) found: raw_text #79

Closed rbuj closed 4 years ago

rbuj commented 4 years ago

Old URI

https://aviationweather.gov/adds/dataserver_current/httpparam?dataSource=metars&requestType=retrieve&format=xml&hoursBeforeNow=3&mostRecent=true&stationString=LEDA&fields=raw_text

New URI

https://aviationweather.gov/adds/dataserver1_3/httpparam?dataSource=metars&requestType=retrieve&format=xml&hoursBeforeNow=3&mostRecent=true&stationString=LEDA&fields=raw_text

Test: Update weather info on mateweather-applet

msg->response_body->data contains raw_text element now

(gdb) set print elements 0
(gdb) print "%s", msg->response_body->data
$3 = 0x16464e0 "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\r\n<response xmlns:xsd=\"http://www.w3.org/2001/XMLSchema\" xmlns:xsi=\"http://www.w3.org/2001/XML-Schema-instance\" version=\"1.2\" xsi:noNamespaceSchemaLocation=\"http://aviationweather.gov/adds/schema/metar1_2.xsd\">\r\n  <request_index>15318</request_index>\r\n  <data_source name=\"metars\" />\r\n  <request type=\"retrieve\" />\r\n  <errors />\r\n  <warnings />\r\n  <time_taken_ms>13</time_taken_ms>\r\n  <data num_results=\"1\">\r\n    <METAR>\r\n      <raw_text>LEDA 291130Z AUTO 18005KT CAVOK 27/12 Q1016</raw_text>\r\n    </METAR>\r\n  </data>\r\n</response>\r\n\r\n"
(gdb) 

metar

Closes #80

muktupavels commented 4 years ago

I would not consider this as fix without having more info... That parameter was used to limit returned data. Now it returns more data, that wont be used...

vkareh commented 4 years ago

@muktupavels has a good point. Regardless of the amount of data, dataserver_current seems like a break in API, whereas dataserver1_3 seems like the actual locked-in version of the API that we've been using up to now (or compatible enough).

I would estimate that dataserver1_3 won't change but dataserver_current can change spec without notice, as it's likely a pointer to whatever new version of their API. Maybe it makes sense to lock in the 1.3 version

rbuj commented 4 years ago

The current server of AWC is TDS 1.3 (January 2010), which uses metar1_2.xsd. It should be available at the following URLs:

  1. http://aviationweather.gov/dataserver_current/current/
  2. http://aviationweather.gov/adds/dataserver1_3/current/

The 1st server doesn't accept fields=raw_text in Fields constraint, by removing it from the soap request, the response contains the METAR element, what includes raw_text.

    <xsd:element name="METAR">
        <xsd:complexType>
            <xsd:sequence >
                <xsd:element ref="raw_text" minOccurs="0" />
                <xsd:element ref="station_id" minOccurs="0"/>
                                <xsd:element ref="observation_time" minOccurs="0"/>
                                <xsd:element ref="latitude" minOccurs="0"/>
                <xsd:element ref="longitude" minOccurs="0"/>
                <xsd:element ref="temp_c" minOccurs="0"/>
                <xsd:element ref="dewpoint_c" minOccurs="0"/>
                <xsd:element ref="wind_dir_degrees" minOccurs="0"/>
                <xsd:element ref="wind_speed_kt" minOccurs="0"/>
                <xsd:element ref="wind_gust_kt" minOccurs="0"/>
                <xsd:element ref="visibility_statute_mi" minOccurs="0"/>
                <xsd:element ref="altim_in_hg" minOccurs="0"/>
                <xsd:element ref="sea_level_pressure_mb" minOccurs="0"/>
                <xsd:element ref="quality_control_flags" minOccurs="0"/>
                <xsd:element ref="wx_string" minOccurs="0"/>
                <xsd:element ref="sky_condition" minOccurs="0" maxOccurs="4"/>
                <xsd:element ref="flight_category" minOccurs="0"/>
                <xsd:element ref="three_hr_pressure_tendency_mb" minOccurs="0"/>
                <xsd:element ref="maxT_c" minOccurs="0" />
                <xsd:element ref="minT_c" minOccurs="0" />
                <xsd:element ref="maxT24hr_c" minOccurs="0" />
                <xsd:element ref="minT24hr_c" minOccurs="0"/>
                <xsd:element ref="precip_in" minOccurs="0" />
                <xsd:element ref="pcp3hr_in" minOccurs="0"/>
                <xsd:element ref="pcp6hr_in" minOccurs="0"/>
                <xsd:element ref="pcp24hr_in" minOccurs="0"/>
                <xsd:element ref="snow_in" minOccurs="0"/>
                <xsd:element ref="vert_vis_ft" minOccurs="0"/>
                <xsd:element ref="metar_type" minOccurs="0" />
                <xsd:element ref="elevation_m" minOccurs="0" />
            </xsd:sequence>
        </xsd:complexType>
    </xsd:element>

I have no objection to change the URI base of the request keeping the fields constraint to get only that element:

    <xsd:element name="raw_text" type="xsd:string"/>
raveit65 commented 4 years ago

Big thank you for fast reaction to @rbuj Guys, weather data is back! Do we need the PR?

rbuj commented 4 years ago

The AWC SOAP WS is back with the same conditions in both URIs, ie no changes need to be made in the request.

rbuj commented 4 years ago

Apparently it's still necessary for getting the weather info from METAR database

raveit65 commented 4 years ago

@rbuj Do you think that your proposed code change would work with old and new behaviour from AWC ? Sadly, we don't know if this is a temporary issue by AWC or a change of their API.

vkareh commented 4 years ago

I still think that moving to dataserver1_3 might be a good idea. I don't have any inside information, but from poking around at the website I can only assume that dataserver_current is the equivalent to a symlink to the latest version, which used to be 1.3, but now it's not?

If we want to be more paranoid, we can do both and change to dataserver1_3 and drop the fields=raw_text from the request...

N0rbert commented 4 years ago

What is really bad - this fix is needed for all Ubuntu versions. Did you tried to contact aviationweather.gov admins to ask them changing URL back? It will be safer to not change the code of libmateweather internals, I think.

What should we do if they change the URL again? And anyway it would be good to have some configuration place for weather URL (like dconf, gsettings) to change it without recompilation.

raveit65 commented 4 years ago

What is really bad - this fix is needed for all Ubuntu versions.

Wrong, it is really bad that all Distros need this!

N0rbert commented 4 years ago

I tried to write an e-mail to NOAA and AviationWeather about reverting their API changes. Will inform you on any reply. The contact form https://aviationweather.gov/contact does not work, shows error about their proxy server. The API seems to be documented at https://aviationweather.gov/dataserver .

raveit65 commented 4 years ago

Thank you. I am testing this PR since one day now, currently they didn't fallback to old behaviour.

N0rbert commented 4 years ago

Got answer from NOAA, they are currently running on their backup system. I invited Daniel from NOAA to this pull-request. And wrote to him that we want to have old URL/API back.

devietor commented 4 years ago

I'm the lead web developer at the Aviation Weather Center. I'm trying to get a handle on the change that cause the weather download to fail.

I've noticed that our backup ADDS Dataserver is having troubles with the field specifier. I'm looking into that problem. It appears from the URLs listed above that datasever1_3 works but dataserver_current doesn't. That's good information.

devietor commented 4 years ago

I'm not sure why the dataserver_current and dataserver1_3 are working differently. They're symlinks to the same Tomcat .war file. But they are different. I did notice that the war file on that server is older than we're using on the production server. But while it's operational, I can't change it to test this theory.

The dataserver_current is the preferred link but we keep dataserver1_3 and dataserver1_3_0 active. The latter two work fine on the backup server.

We will be on the backup server until probably next Monday or Tuesday. The server work at the Weather Service web farm will be going through this Friday. We might switch back on Friday but it's looking more like Monday.

Here are direct links:

current production: https://aviationweather.cp.ncep.noaa.gov/adds/dataserver_current/httpparam?dataSource=metars&requestType=retrieve&format=xml&hoursBeforeNow=3&mostRecent=true&stationString=LEDA&fields=raw_text

current backup: https://aviationweather-mo.weather.gov/adds/dataserver_current/httpparam?dataSource=metars&requestType=retrieve&format=xml&hoursBeforeNow=3&mostRecent=true&stationString=LEDA&fields=raw_text

We are in the process of upgrading our servers from Redhat 6 to 7. The new RH7 servers are:

primary: https://aviationweather-cprk.ncep.noaa.gov/adds/dataserver_current/httpparam?dataSource=metars&requestType=retrieve&format=xml&hoursBeforeNow=3&mostRecent=true&stationString=LEDA&fields=raw_text

backup: https://aviationweather-bldr.ncep.noaa.gov/adds/dataserver_current/httpparam?dataSource=metars&requestType=retrieve&format=xml&hoursBeforeNow=3&mostRecent=true&stationString=LEDA&fields=raw_text

satoshi commented 4 years ago

What's holding up merging this change in? There are two items "Pending" but if you look at the Details, they're green.

vkareh commented 4 years ago

@satoshi - the pipeline has nothing to do with it. If you read the thread, you will notice that there's still an issue in the API that we use to fetch weather data, and they are performing work on their servers that will extend possibly until Monday/Tuesday.

satoshi commented 4 years ago

@vkareh I read the thread and understand that the server work is still ongoing; however I thought this change would address the issue regardless.

you will notice that there's still an issue in the API that we use to fetch weather data

I don't know what you're talking about. You said yourself "I still think that moving to dataserver1_3 might be a good idea" which this change would do.

raveit65 commented 4 years ago

I don't know what you're talking about. You said yourself "I still think that moving to dataserver1_3 might be a good idea" which this change would do.

Nope, fixing all distros is a bad idea! I prefer that it should work like before.

If you can't wait a few day, you need to patch your installation for yourself. ....... and revert it some days later for yourself :)

vkareh commented 4 years ago

@satoshi

You said yourself

I know what I said, and then we had someone from AWC confirm that there is an issue and that they are working on their servers. So we've decided to hold off on this patch.

Even if we merge this now, unless your distro maintainer patches it immediately, you will still experience the issue. If you maintain your own distro, then you can patch it regardless of whether we merge or not: https://github.com/mate-desktop/libmateweather/pull/79.patch

Again, read the thread and please refrain from edging our developers unnecessarily.

satoshi commented 4 years ago

@vkareh I'm not edging anybody. What was missing is this communication: "So we've decided to hold off on this patch." If that's the case it's fine to wait the upstream to complete the server work.

devietor commented 4 years ago

The latest is that there is more server work to be done early Monday. So we won't be going back to the primary until Monday afternoon at the earliest.

As for the change of URL, that's up to you. The dataserver_current, dataserver1_3 and dataserver1_3_0 links should all work the same. That's how it's designed. Of course, that's in a perfect world and our current backup server is not behaving as designed.

If you want to work a primary and backup URL, let me know. That's partly why I gave you those new URLs. The schedule right now is to upgrade to the RHEL7 servers on June 23.

N0rbert commented 4 years ago

It is nearly impossible to patch all current Ubuntu MATE (i.e. 18.04 LTS, 19.10 and 20.04 LTS) and other distros which ship MATE to include new URL or API. From user point-of-view I can say that it would be really good to get weather informer inside MATE Clock Applet working as before using old URL. I hope @devietor can solve the problem this way :)

kikislater commented 4 years ago

Why could we not change url in dconf editor ?

satoshi commented 4 years ago

@raveit65 Could you elaborate why you're downvoting @kikislater 's suggestion? It doesn't have to be a solution to this particular issue at hand, but something to consider long-term so if something like this happens again, the user can update via dconf and call it a day.

raveit65 commented 4 years ago

LOL, do you really think this is a voting like a democratic election?

And why not using weather infos with your web browser for a few days, instead of asking for unneeded developer extra work without doing a donation? We are doing this all in our spare time. So calm down, in a few days everything will work as normal.

vkareh commented 4 years ago

The problem with updating the URL in gsettings/dconf is that this is not something that you can just drop-in and it will work. We query a specific URL in a specific way and it returns a specific type of response.

If you were to just replace the AWC URL by something else it will not work. It's a different API.

If you want that functionality, you would need to change how libmateweather works entirely and support different specific weather providers. It still would not support custom URLs, but would allow a selection. It might be a good thing to do at some point.

Feel free to submit a feature request for this behavior and let's move that conversation over there. I would like to keep this thread on track regarding the specific issue with the AWC server.

kikislater commented 4 years ago

Ok, sorry, it was just a suggestion ... thank you for complete answer @raveit65 and @vkareh I don't have the solution but thanks to @vkareh patch, I upgrade my archlinux with this

satoshi commented 4 years ago

@raveit65 I was just curious why you didn't like the dconf idea that's all, and your stance is reasonable. Thank you @vkareh for the explanation as well.

EDIT: just to be clear, by URL in dconf I was talking about a very minor endpoint change (dataserver_current vs dataserver1_3) - still depending on the AWC server and using the same API. It is a feature request nonetheless, so I'll see if I will file one as such.

devietor commented 4 years ago

The latest is that the work to be done on the servers scheduled for today has been postponed until at least Wednesday thanks to Tropical Storm Cristobal. I have also learned that there is more work scheduled for Friday that might force us to go to backup again. Argh!

I made a request to go back to production this morning but management seems to want to stay on the backup. I wish I had better news.

satoshi commented 4 years ago

Unblocked myself by manually hacking the shared object. Now weather is back, I can weather Tropical Storm Cristobal. Thanks everyone! :smile:

devietor commented 4 years ago

It's not clear if we'll go off backup. This is a management call. We have a timeline to implement the new RHEL 7 server. It's now set for June 23rd. The old URL will work on the RH7 server if you wish to wait 2 weeks. Until then, we may stay on our backup server. That's not good for this issue.

If you want to change the URL to dataserver1_3, go for it. That will still work when we upgrade to the new RH7 server.

Sorry for the inconvenience.

procule commented 4 years ago

@raveit65 I was just curious why you didn't like the dconf idea that's all, and your stance is reasonable. Thank you @vkareh for the explanation as well.

EDIT: just to be clear, by URL in dconf I was talking about a very minor endpoint change (dataserver_current vs dataserver1_3) - still depending on the AWC server and using the same API. It is a feature request nonetheless, so I'll see if I will file one as such.

Just to say that I agree with you. Having a hardcoded URL in any source code is almost always a bad idea. We are seeing the consequences here, impacting mostly all distros. I understand that the applet is specifically made for that API, but as @devietor said, there are different endpoints.

raveit65 commented 4 years ago

This will happen before next major release. https://github.com/mate-desktop/mate-panel/pull/1075 Than you can discuss your ideas with gnome devs.

N0rbert commented 4 years ago

Personally I do not like GNOME parts to be included to MATE. This will lead to lose of traditional desktop. We can't predict what GNOME developers will remove next time. Dependance on non-stable GNOME' developer minds is not good idea. Also I do not think we need Vala on MATE, as GWeather depends on Vala. So I think that defining correct aviationweather URL in the dconf/gsettings (while using libmateweather) is better solution.

All current Ubuntu MATE (and others, likely) systems may be temporarily fixed by below one-liner for binary patching: sudo sed -i 's|https://www.aviationweather.gov/adds/dataserver_current/httpparam|https://www.aviationweather.gov/adds/dataserver1_3/httpparam\x0\x0\x0\x0\x0|' $(readlink -f /usr/lib/x86_64-linux-gnu/libmateweather.so.1)

Updated, above snippet to keep symlink. Thanks, @satoshi .

bparees commented 4 years ago

All current Ubuntu MATE (and others, likely) systems may be temporarily fixed by below one-liner for binary patching: sudo sed -i 's|https://www.aviationweather.gov/adds/dataserver_current/httpparam|https://www.aviationweather.gov/adds/dataserver1_3/httpparam\x0\x0\x0\x0\x0|' /usr/lib/x86_64-linux-gnu/libmateweather.so.1

confirmed this works on fedora32 changing the path to /usr/lib64/libmateweather.so.1.6.9 (could probably target the .so.1 symlink instead, too)

thanks!

satoshi commented 4 years ago

Yeah I did something similar to what @N0rbert said. The only issue I am having is, I backed up the original so file with .bak extension; but somehow, something updates my symlink to point to the backup (original) file instead of patched libmateweather.so.1.6.9 file. I moved the backup file elsewhere and re-symlinked; I'll see what happens.

satoshi commented 4 years ago

@N0rbert do you want to raise your concern regarding gweather with the upstream (https://github.com/mate-desktop/mate-panel/pull/1075) before the change gets merged?

On the topic of symlinking, my symlink appeared to have survived after I moved the original SO file out of /usr/lib/x86_64-linux-gnu.

ssrublev commented 4 years ago

confirmed this works on fedora32 changing the path to /usr/lib64/libmateweather.so.1.6.9 (could probably target the .so.1 symlink instead, too)

Worked for me at fedora32, thanks!

highflux7 commented 4 years ago

For us still riding the shortbus, here's the nearly identical 1-line binary patch for i386 MATE:

sudo sed -i 's|https://www.aviationweather.gov/adds/dataserver_current/httpparam|https://www.aviationweather.gov/adds/dataserver1_3/httpparam\x0\x0\x0\x0\x0|' $(readlink -f /usr/lib/i386-linux-gnu/libmateweather.so.1)

pepa65 commented 4 years ago

Should I open an issue for this (ie. is this fixable?). I moved to a place that has a METAR/TAF record like this (note the first line) (https://tgftp.nws.noaa.gov/data/observations/metar/decoded/VTCT.TXT):

Station name not available
Jun 15, 2020 - 10:00 AM EDT / 2020.06.15 1400 UTC
Wind: from the SW (220 degrees) at 3 MPH (3 KT):0
Visibility: greater than 7 mile(s):0
Sky conditions: overcast
Temperature: 78 F (26 C)
Dew Point: 75 F (24 C)
Relative Humidity: 88%
Pressure (altimeter): 29.77 in. Hg (1008 hPa)
ob: VTCT 151400Z 22003KT 9999 FEW030 BKN048 OVC090 26/24 Q1008 NOSIG
cycle: 14

This is Chiang Rai, but I can't get the local weather to display through libmateweather...

I would be willing to maintain a little list of station codes and names for the "Station name not available" cases that can be queried.

raveit65 commented 4 years ago

@rbuj Maybe we do something like that? https://gitlab.gnome.org/GNOME/libgweather/-/commit/fa771a8dca7148c23b9e604b4fdb4963fc7d5003

devietor commented 4 years ago

We're still on pace to move to the new servers on June 23.

BTW, we don't manage the decoded data on the TGFTP server. My suspicion is their station database is really old.

https://www.aviationweather.gov/metar/data?ids=VTCT&format=decoded&hours=0&taf=off&layout=on

METAR for: | VTCT (Chiang Rai Intl, --, TH) Text: | VTCT 191900Z 20004KT 9999 FEW015 25/25 Q1008 NOSIG Temperature: | 25.0°C ( 77°F) Dewpoint: | 25.0°C ( 77°F) [RH = 100%] Pressure (altimeter): | 29.76 inches Hg (1008.0 mb) Winds: | from the SSW (200 degrees) at 5 MPH (4 knots; 2.1 m/s) Visibility: | 6 or more sm (10+ km) Ceiling: | at least 12,000 feet AGL Clouds: | few clouds at 1500 feet AGL

pepa65 commented 4 years ago

I know there are some queries that do return the proper station name. There must be a way to handle any "Station name not available" cases within libmateweather. This has not worked for many years (forever?).

devietor commented 4 years ago

We upgraded to RHEL7 today at 3PM EDT. I see I have data back in my Mate weather app.

If you're interested, I also maintain a database of city locations and names for METAR observation sites. I update it about every 2 months so we have the latest information for our website.

pepa65 commented 4 years ago

Yes, weather's back here too, but no way to add my current location... (Even though we live 5 minutes away from an international airport...)

vkareh commented 4 years ago

@pepa65

no way to add my current location

That's an unrelated issue. There are several issues filed for that already.

@devietor

If you're interested, I also maintain a database of city locations and names for METAR observation sites

Thanks so much for reporting back on your server upgrade! It all works now on my side! :) I am interested in the city locations, how do we access that? We currently keep a hand-edited list, but it's a pain to maintain...

raveit65 commented 4 years ago

I guess this can close now, or not? @devietor Thanks a lot.

rbuj commented 4 years ago

I close it because it's no longer necessary.

devietor commented 4 years ago

We upgraded to RHEL7 today at 3PM EDT. I see I have data back in my Mate weather app.

If you're interested, I also maintain a database of city locations and names for METAR observation sites. I update it about every 2 months so we have the latest information for our website.

@pepa65

no way to add my current location

That's an unrelated issue. There are several issues filed for that already.

@devietor

If you're interested, I also maintain a database of city locations and names for METAR observation sites

Thanks so much for reporting back on your server upgrade! It all works now on my side! :) I am interested in the city locations, how do we access that? We currently keep a hand-edited list, but it's a pain to maintain...

I'm trying to get an update on posting the latest surface observation table. The one on our website is from last November. I just did an update at the beginning of June and I'm trying to get that released.

https://www.aviationweather.gov/docs/metar/sao.cty

I'll post again when this is updated. I'm hoping before the end of July.