elixir-cloud-aai / proWES

Proxy service for injecting middleware into GA4GH WES requests
Apache License 2.0
3 stars 5 forks source link

ci: add type checks #88

Closed JaeAeich closed 9 months ago

JaeAeich commented 10 months ago

IMPORTANT: Please create an issue before filing a pull request! Changes need to be discussed before proceeding. Please read the contribution guidelines.

Details This PR add type checking using mypy as stated in issue #77

Closing issues

Fixes issue - #77

JaeAeich commented 10 months ago

@uniqueg Is this good (in right dir)? I still need to narrow it down a little as I have ignored import errors in mypy.ini which I think needs a little more work.

JaeAeich commented 10 months ago

Hey @uniqueg, unfortunately mypy doesn't check that deep, this is in regard to your suggestion to create a validator func. So the other solution would be something like this:

        assert document_stored.wes_endpoint is not None, "No WES endpoint available."
        assert (
            document_stored.wes_endpoint.base_path is not None
        ), "WES endpoint does not have base_path."

        if document_stored.task_id is None:
            raise WesEndpointProblem

Where WesEndpointProblem is

....
class WesEndpointProblem(NotFound):
    """No/few reuirements provided for WES endpoint."""

exceptions={
   ....
    WesEndpointProblem: {
        "message": "No/few reuirements provided for WES endpoint.",
        "code": "500",
    }
}

yeah that mean I gotta do it in two place ie, for document, updated_document and in track_run_progress. Please do tell me if there is better way to do it. PS: Not yet ready for reviews.

uniqueg commented 9 months ago

Hey @uniqueg, unfortunately mypy doesn't check that deep, this is in regard to your suggestion to create a validator func. So the other solution would be something like this:

        assert document_stored.wes_endpoint is not None, "No WES endpoint available."
        assert (
            document_stored.wes_endpoint.base_path is not None
        ), "WES endpoint does not have base_path."

        if document_stored.task_id is None:
            raise WesEndpointProblem

Where WesEndpointProblem is

....
class WesEndpointProblem(NotFound):
    """No/few reuirements provided for WES endpoint."""

exceptions={
   ....
    WesEndpointProblem: {
        "message": "No/few reuirements provided for WES endpoint.",
        "code": "500",
    }
}

yeah that mean I gotta do it in two place ie, for document, updated_document and in track_run_progress. Please do tell me if there is better way to do it. PS: Not yet ready for reviews.

Looks fine to me, not too messy :)

uniqueg commented 9 months ago

Sorry, only saw now that you were not ready for review. Gave you one anyway :laughing:

But it seems to me that you addressed at least most of my previous issues. Please make sure to resolve the corresponding conversations so that we can better keep track of what's still left to do.

JaeAeich commented 9 months ago
ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
foca 0.12.1 requires pymongo==3.10.1, but you have pymongo 3.13.0 which is incompatible.

I am disabling all error for missing imports, when foca update pymongo to 3.11 we can remove it. This is because pymongo-stubs is compatible with Python >=3.6 and [PyMongo](https://pypi.org/project/pymongo/) >=3.11,<4.0.

JaeAeich commented 9 months ago

@uniqueg Please review the PR. I have made the requested changes :+1: