-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add new APIs to catalogue #265
Conversation
Codecov Report
@@ Coverage Diff @@
## master #265 +/- ##
==========================================
- Coverage 91.53% 90.16% -1.38%
==========================================
Files 32 32
Lines 1749 1749
==========================================
- Hits 1601 1577 -24
- Misses 148 172 +24 |
Was issue #250 already implemented so that API limits were unneeded in API catalogue? For now I updated them manually but if they can be safely removed I can do that as well |
Hreat work pyry. There seem to be some issues on the APIs that fails. Maybe move checking the api catalogue to only tests run on test branch. |
I fixed the error on test coverage
by changing pxac[[2]] and pxacgh[[2]] to pxac[[1]] and pxacgh[[1]], effectively making the test compare the first item on API list (SCB). There was the following NOTE on R-CMD-check:
The failed test on R-CMD-CHECK was related to running pxweb.sh from tests_bash folder. There indeed seems to be a problem related to permissions:
What do you mean by "move checking the api catalogue to only tests run on test branch"? |
Hmm. This is a new thing. The same code has worked before. Could you try to add chmod? See below. |
I could do that. But at least when I tried running tests_bash/pxweb.R on my computer it took a very long time to do the
phase, surely running something like that on Github Actions is not a good idea? Actually some APIs were so big that they timed out before
was finished. |
I did a new PR #267 that updates the R-CMD-check workflows to more modern ones. I did not include rows 90-99 from the current workflow:
I'm sure the workflow could be configured to check --as-cran as before but I'm not sure if that's necessary at all steps. Additionally, the pxweb.sh section is now completely removed, as I was not totally sure why such a time-consuming script should be run every time updates are made. |
I added the other PR so I close this one npw. Feel free to reopen iof this was incorrect |
Added new APIs to api.json. Some databases created with pxweb technology didn't have API enabled or only had test api so they were not added. Some APIs had some problems:
Entities that did not have API available:
National Statistical Service of the Republic of Armenia
Nordic Health and Welfare Statistics
Entities with broken APIs:
Örebro kommun (old pxweb API version)
EUSTAT (redirects to net-inter-eustat-45.ejgvdns, reported to their helpdesk)
INSTAT, Mali (sometimes fails, sometimes produces an error message, sometimes works, unstable internet connection?)
Icelandic Centre for Retail Studies (px.rsv.is) (no longer existing, timeouts?)
API works fine but produces a warning:
Sundsvall kommun
See issue #254 for a more detailed list and error messages
Also: