Skip to content
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

WFS 2.0 URL building #612

Merged
merged 5 commits into from
Oct 6, 2024
Merged

WFS 2.0 URL building #612

merged 5 commits into from
Oct 6, 2024

Conversation

geographika
Copy link
Contributor

@geographika geographika commented Oct 9, 2019

In WFS 2.0 GetFeature requests currently fail for MapServer URLs containing ?map=path/my.map&
This is the same issue as #533 - and unfortunately I missed that there was also an open pull request at #534 from @samtux
The approach in this pull request is slightly different - it reuses the build_get_url function from utils.

A few other notes:

  • There is a warning about owslib\util.py:561: DeprecationWarning: cgi.parse_qsl is deprecated, use urllib.parse.parse_qsl instead so this has been updated
  • the WFS GetFeature URL building allowed for the doseqparameter to be used (I'm not sure when this applies to WFS requests, but I added it to avoid any breaking changes). It allows for automatic duplicate keys for sequences e.g.
>>> urllib.urlencode({'keys': [1,2]}, doseq=True)
>>> 'keys=1&keys=2'
>>> urllib.urlencode({'keys': [1,2]}, doseq=False)
>>> 'keys=%5B1%2C+2%5D'
  • I 've also added tests for the doseq and URLs with ampersands

@justb4
Copy link
Member

justb4 commented Oct 10, 2019

Test fails for same reason as with my PR #613 for #587 initially:

tests/test_wfs3_pygeoapi.py::test_wfs3_pygeoapi FAILED

has nothing to do with your PR, but a recent change in pygeoapi and its demo server used in tests.

@coveralls
Copy link

Coverage Status

Coverage: 59.054% (-0.01%) from 59.063% when pulling 5eab326 on geographika:url-fix into a365101 on geopython:master.

@geographika geographika marked this pull request as ready for review February 13, 2023 21:25
Copy link

github-actions bot commented Oct 6, 2024

This Pull Request has been inactive for 90 days. In order to manage maintenance burden, it will be automatically closed in 7 days.

@github-actions github-actions bot added the stale label Oct 6, 2024
@coveralls
Copy link

coveralls commented Oct 6, 2024

Coverage Status

coverage: 59.719% (-0.006%) from 59.725%
when pulling 0af980b on geographika:url-fix
into f620405 on geopython:master.

@geographika
Copy link
Contributor Author

Pull request rebased onto latest master branch and all tests passing.

The coveralls check is failing - it reports -0.006% coverage change, but I don't think this is correct - the changes in this pull requests are tested as previously. It also reported -0.2% on another run with the same code.

I ran into a similar issue on MapServer with small changes between runs. We increased the threshold to 1% to avoid this (see MapServer/MapServer#6223). Maybe OWSLib is also required to have 60% coverage?

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

Successfully merging this pull request may close these issues.

4 participants