Skip to content

Reject boolean values for the limit parameter - #2419

Open
Shubham-Padkonde wants to merge 1 commit into
geopython:masterfrom
Shubham-Padkonde:fix/limit-reject-booleans
Open

Shubham-Padkonde wants to merge 1 commit into
geopython:masterfrom
Shubham-Padkonde:fix/limit-reject-booleans

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown
Contributor

get_typed_value turns limit=true into the boolean True, which passed the isinstance(..., int) check because bool is a subclass of int, so evaluate_limit returned True as the limit (and limit=false failed with "should be strictly positive"). Booleans are now rejected with the same "should be an integer" error as other non-integer values.

Overview

Related Issue / discussion

Additional information

Dependency policy (RFC2)

  • I have ensured that this PR meets RFC2 requirements

Updates to public demo

Contributions and licensing

(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to pygeoapi. I confirm that my contributions to pygeoapi will be compatible with the pygeoapi license guidelines at the time of contribution
  • I have already previously agreed to the pygeoapi Contributions and Licensing Guidelines

get_typed_value turns limit=true into the boolean True, which passed the
isinstance(..., int) check because bool is a subclass of int, so
evaluate_limit returned True as the limit (and limit=false failed with
"should be strictly positive"). Booleans are now rejected with the
same "should be an integer" error as other non-integer values.
@Shubham-Padkonde

Copy link
Copy Markdown
Contributor Author

The current Build failure is in PostgreSQL provider setup: all 42 failed cases in #2419 and #2420 raise ModuleNotFoundError: No module named 'psycopg'; 382 tests pass in each run. The failing cases use the shared connection_string fixture with postgresql://..., while the current provider requirements install psycopg2 only. Neither PR changes these provider requirements or fixtures.

Logs: https://github.com/geopython/pygeoapi/actions/runs/36156172783 and https://github.com/geopython/pygeoapi/actions/runs/36156196416.

Would you prefer the shared fixture to specify postgresql+psycopg2://, or to add psycopg 3 support to the provider test environment? I have left that broader driver choice outside these two fixes. Diagnosis prepared with Codex assistance.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant