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

test: Fix databuilder PR unit test import error #1891

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

chonyy
Copy link
Contributor

@chonyy chonyy commented Jun 7, 2022

Summary of Changes

Originally, the databuilder unit test will fail because of the fastAPI version problem. This blocked all the databuilder related PRs. Fast API is required by Feast.
An example of the failing workflow: https://github.com/amundsen-io/amundsen/runs/6646082819
The log looks like

tests/unit/extractor/test_feast_extractor.py:13: in <module>
    from databuilder.extractor.feast_extractor import FeastExtractor
databuilder/extractor/feast_extractor.py:7: in <module>
    from feast import FeatureStore, FeatureView
/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/feast/__init__.py:13: in <module>
    from .feature_store import FeatureStore
/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/feast/feature_store.py:[38](https://github.com/amundsen-io/amundsen/runs/6646082819?check_suite_focus=true#step:5:39): in <module>
    from feast import feature_server, flags, flags_helper, utils
/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/feast/feature_server.py:3: in <module>
    from fastapi import FastAPI, HTTPException, Request
/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/fastapi/__init__.py:7: in <module>
    from .applications import FastAPI as FastAPI
/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/fastapi/applications.py:4: in <module>
    from fastapi import routing
/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/fastapi/routing.py:22: in <module>
    from fastapi.datastructures import Default, DefaultPlaceholder
/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/fastapi/datastructures.py:3: in <module>
    from starlette.datastructures import URL as URL  # noqa: F[40](https://github.com/amundsen-io/amundsen/runs/6646082819?check_suite_focus=true#step:5:41)1
/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/starlette/datastructures.py:8: in <module>
    from starlette.concurrency import run_in_threadpool
/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/starlette/concurrency.py:10: in <module>
    from typing_extensions import ParamSpec
E   ImportError: cannot import name 'ParamSpec' from 'typing_extensions' (/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/site-packages/typing_extensions.py)

According to fastapi/fastapi#4877 and fastapi/fastapi#4868 (comment) , the problem is fixed in fastapi==0.77.0. However, doing fastapi>=0.77.0 seems to be incompatible with other packages, this is a test run of it. That's why I decided to just skip the troublesome versions.

Tests

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes.
  • PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does

@boring-cyborg boring-cyborg bot added the area:databuilder From databuilder folder label Jun 7, 2022
@chonyy chonyy changed the title ci: Fix databuilder unit test import error ci: Fix databuilder PR unit test import error Jun 7, 2022
Signed-off-by: Tony Chou <tcheon8788@gmail.com>
@chonyy chonyy force-pushed the fix-databuilder-unit-test-import branch from a2e1cd5 to ed8788e Compare June 7, 2022 19:04
@chonyy chonyy changed the title ci: Fix databuilder PR unit test import error test: Fix databuilder PR unit test import error Jun 7, 2022
@dkunitsk dkunitsk merged commit b37886d into amundsen-io:main Jun 7, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Jun 7, 2022

Awesome work, congrats on your first merged pull request!

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

Successfully merging this pull request may close these issues.

2 participants