search

MHKiT-Software / MHKiT-MATLAB

MHKiT-MATLAB provides the marine renewable energy (MRE) community tools for data processing, visualization, quality control, resource assessment, and device performance.
https://mhkit-software.github.io/MHKiT/
BSD 3-Clause "New" or "Revised" License
15 stars 23 forks source link

Bug fix io request data #78

Closed Matthew-Boyd closed 2 years ago

Matthew-Boyd commented 2 years ago

Closes #72

All USGS, NOAA and NBDC data requests work and their tests pass.

@rpauly18 , Please let me know if you would like any changes or improvements to my coding style and scope. I can improve the robustness, error handling, modularity, etc., but this is currently the balance I've chosen with my time. Thanks.

rpauly18 commented 2 years ago

@Matthew-Boyd I am getting an error in test_ndbc_available_data. I have tried running twice, but no change in the error. Did you see this error at all in your testing?

Error using webread (line 122) The connection to URL 'https://www.ndbc.noaa.gov/data/historical/swden/' timed out after 5.000 seconds. The reason is "Waiting for response header". Perhaps the server is not responding or weboptions.Timeout needs to be set to a higher value.

Matthew-Boyd commented 2 years ago

_"I am getting an error in test_ndbc_availabledata. I have tried running twice, but no change in the error. Did you see this error at all in your testing?"

I saw a similar error only a couple times, so it was very infrequently. I ran the test a bunch last night and a bunch this morning, both on and off the VPN, and wasn't able to replicate it. I was thinking about adding a catch and automatic retry, which would be nice, but it seems there's a bigger problem if it doesn't ever work for you.

rpauly18 commented 2 years ago

_"I am getting an error in test_ndbc_availabledata. I have tried running twice, but no change in the error. Did you see this error at all in your testing?"

I saw a similar error only a couple times, so it was very infrequently. I ran the test a bunch last night and a bunch this morning, both on and off the VPN, and wasn't able to replicate it. I was thinking about adding a catch and automatic retry, which would be nice, but it seems there's a bigger problem if it doesn't ever work for you.

@Matthew-Boyd I will try some more today. If I can get it to work, an automatic retry would definitely be nice.

rpauly18 commented 2 years ago

@Matthew-Boyd I was able to get the test working. Call it out if you already added it, but the automatic retry would be nice. Then I think this is ready to merge

Matthew-Boyd commented 2 years ago

@rpauly18 I added the automatic retries, although after hundreds of queries each I wasn't able to get a failure to fully test it. Figures... Please review or merge.