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

Switch to own packaged version of pygtfs #15040

Merged
merged 3 commits into from
Jul 15, 2018
Merged

Conversation

andrey-git
Copy link
Contributor

Description:

Switch to own packaged version of pygtfs

This is the diff by @robbiet480 : jarondl/pygtfs@master...robbiet480:fix-sqlite-multithreading

According to Robbie it is needed and was rejected upstream.

Related issue (if applicable): #7069

@ghost ghost assigned andrey-git Jun 18, 2018
@ghost ghost added the in progress label Jun 18, 2018
@robbiet480
Copy link
Member

FYI, this is needed because SQLAlchemy complains if you attempt to use the same connection across threads. Here's the PR.

Thanks for handling this @andrey-git!

@andrey-git andrey-git removed their assignment Jul 2, 2018
@robbiet480
Copy link
Member

robbiet480 commented Jul 2, 2018

@andrey-git In a fit of insomnia i've taken another look at this and have determined a way for us to use the mainline version of PyGTFS. The only issue is that the project uses setuptools-scm which throws an error when trying to install a specific commit like https://github.com/jarondl/pygtfs/archive/f91ca234c19286e634b2d8b1ab27c78ad7b277f4.zip#pygtfs:

LookupError: setuptools-scm was unable to detect version for '/private/var/folders/6d/4n3mhsvx3ts7fmx4jb2zqw5r0000gn/T/pip-klo4fhu2-build'.

    Make sure you're either building from a fully intact git repository or PyPI tarballs. Most other sources (such as GitHub's tarballs, a git checkout without the .git folder) don't contain the necessary metadata and will not work.

    For example, if you're using pip, instead of https://github.com/user/proj/archive/master.zip use git+https://github.com/user/proj.git#egg=proj

Got any ideas to work around this? I think it would be better in practice to use the mainline dep instead of our own fork but obviously this won't fly if we can't lock the dep to a specific commit.

We could also just use the PyPI published version 0.1.3. There's been minimal changes since that tag was cut in April 2016.

BTW, the change that allows this is to construct our own SQLite URL like sqlite:////Users/robbiet480/Repos/HomeAssistant/home-assistant/testConfig/gtfs/bart.sqlite?check_same_thread=False. That check_same_thread=False does the same thing as my forked repo commit.

@andrey-git
Copy link
Contributor Author

We want to avoid non pypi dependencies.

If you can make it work with existing pypi package - awesome!

@andrey-git
Copy link
Contributor Author

This is now the last non-pypi package.
I propose we merge this PR and then explore the possibility to use the upstream package.

@andrey-git
Copy link
Contributor Author

@robbiet480 Kind ping on this :)

@balloob balloob merged commit 5995c6a into home-assistant:dev Jul 15, 2018
@ghost ghost removed the in progress label Jul 15, 2018
@balloob balloob mentioned this pull request Jul 20, 2018
@soraxas
Copy link
Contributor

soraxas commented Jul 28, 2018

Hi!
The pypi upstream repo is outdated and does not work with newer gtfs format. Namely, I just encounter the the lib reporting invalid column type and exited, but in fact the upstream had already updated to deal with it.

The frozen fork here checked against only one route_types but the upstream gtfs has updated with an extended route types.
Probably update the frok with the newer upstream updates?

@frenck
Copy link
Member

frenck commented Jul 29, 2018

@soraxas Please don't use merged PRs as a place for discussion. Please create an issue, or, use one of our other channels.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Jul 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants