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

Bump supported Python version to 3.7 #1504

Merged
merged 1 commit into from
Apr 26, 2021

Conversation

tsotnet
Copy link
Collaborator

@tsotnet tsotnet commented Apr 26, 2021

Signed-off-by: Tsotne Tabidze tsotne@tecton.ai

What this PR does / why we need it: Currently, setup.py incorrectly supports Python 3.6. Bumping the version to 3.7.

Which issue(s) this PR fixes:

Fixes #1502

Does this PR introduce a user-facing change?:

NONE

Signed-off-by: Tsotne Tabidze <tsotne@tecton.ai>
@tsotnet
Copy link
Collaborator Author

tsotnet commented Apr 26, 2021

/test test-telemetry

@feast-ci-bot
Copy link
Collaborator

@tsotnet: The /retest command does not accept any targets.
The following commands are available to trigger jobs:

  • /test test-core-and-ingestion
  • /test test-serving
  • /test test-java-sdk
  • /test test-telemetry
  • /test test-golang-sdk

Use /test all to run the following jobs:

  • test-telemetry

In response to this:

/retest test-telemetry

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@woop
Copy link
Member

woop commented Apr 26, 2021

/lgtm

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tsotnet, woop

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot feast-ci-bot merged commit 1efc53b into feast-dev:master Apr 26, 2021
woop pushed a commit that referenced this pull request Apr 27, 2021
Signed-off-by: Tsotne Tabidze <tsotne@tecton.ai>
@Joostrothweiler
Copy link
Contributor

Joostrothweiler commented Jun 1, 2021

Are there other reasons besides the issue described in #1502 that makes us no longer want to support Python 3.6? We are currently running Python 3.6 (with Feast 0.8) and it seems to me could work around the issue described quite easily by casting the _db_path to a string as described by @chappers in the issue.

Did a quick test run and the following works on Python 3.6 for feast init & feast apply:

class LocalProvider(Provider):
    _db_path: Path

    def __init__(self, config: RepoConfig, repo_path: Path):
        ...
        if local_path.is_absolute():
            self._db_path = str(local_path)
        else:
            self._db_path = str(repo_path.joinpath(local_path))
        self.offline_store = get_offline_store_from_config(config.offline_store)
...

@8bit-pixies
Copy link
Contributor

Just an FYI; the rationale was provided here: #1503 (comment)

@Joostrothweiler
Copy link
Contributor

Thanks @chappers for pointing that out.

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

Successfully merging this pull request may close these issues.

Python 3.6 Local Mode breaks due to usage of pathlib.Path and sqlite3
5 participants