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

Add JupyterHub #540

Merged
merged 2 commits into from
May 4, 2016
Merged

Add JupyterHub #540

merged 2 commits into from
May 4, 2016

Conversation

minrk
Copy link
Member

@minrk minrk commented May 4, 2016

JupyterHub only runs on Python 3, non-Windows. Hopefully I specified that correctly.

Includes pamela, a dependency of jupyterhub.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/jupyterhub, recipes/pamela) and found it was in an excellent condition.

@minrk
Copy link
Member Author

minrk commented May 4, 2016

Should I be setting python_noarch: true on pure Python packages like these?


source:
fn: pamela-{{ version }}.tar.gz
url: https://pypi.python.org/packages/43/d1/21403bbafd8729d441ba45e145810f4b7a1f339718827de336cc7246c450/pamela-{{ version }}.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are preferring the pypi.io URLs instead of the new (ugly) ones:

https://pypi.io/packages/source/p/pamela/pamela-{{ version }}.tar.gz

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is better! Fixed to use the pypi.io URLs

@ocefpaf
Copy link
Member

ocefpaf commented May 4, 2016

JupyterHub only runs on Python 3, non-Windows. Hopefully I specified that correctly.

LGTM. You can add [skip appveyor] to the commit message to prevent it from starting.

@ocefpaf
Copy link
Member

ocefpaf commented May 4, 2016

Should I be setting python_noarch: true on pure Python packages like these?

That is a good question. Right now I would say no as our framework builds all the archs and platform, so that option won't get used. Also, I believe that some packages, even though they are pure python, might misbehave when that option is used. (Ping @msarahan I do not know that really happened in suds-jurko, but I recall it had something to do with the no_arch option.)

@msarahan
Copy link
Member

msarahan commented May 4, 2016

Should be fine here. Suds-jurko was not good because it depended on 2to3 to support both pythons. Not an issue here.

Fine to try noarch, but I'd still test very carefully to make sure nothing dumb happens.

@ocefpaf
Copy link
Member

ocefpaf commented May 4, 2016

Fine to try noarch, but I'd still test very carefully to make sure nothing dumb happens.

I guess that to really take advantage of that feature we need to change the way we configure our CIs.

@minrk
Copy link
Member Author

minrk commented May 4, 2016

I'll leave the noarch frontier for someone more adventurous, then.


build:
number: 0
skip: true # [win or py2k]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ocefpaf
Copy link
Member

ocefpaf commented May 4, 2016

@minrk are you planning on more changes or should I merge this?

build:
number: 0
skip: true # [win or py2k]
script: pip install --no-deps .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost shudder to ask, but using 1 from ( #528 ) is the standard. So, should we change it to python setup.py install --single-version-externally-managed --record=record.txt until the standard changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakirkham we already have pip install --no-deps . in a few feedstocks and it is virtually the same as python setup.py install --single-version-externally-managed --record=record.txt. I am inclined to leave it as is and start the move to pip slowly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

single-version-externally-managed fails on both of these packages with:

error: option --single-version-externally-managed not recognized

because neither package imports setuptools unconditionally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakirkham that is 👍 to pip install --no-deps . as we can the same behavior even without setuptools.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ocefpaf, I have no problem changing the standard if something is more effective, but it would be good if we can get everyone to agree. The issue we have now is if everyone is building each package differently it is more likely some issues creep in or we miss something during a review due to all of the acceptable ways to do something. No one wants broken packages.

...because neither package imports setuptools unconditionally.

Ah, ok. Yeah, what we had been doing is a normal install without the options then. Though I agree this is tedious to discover which way to go.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it would be good if we can get everyone to agree

Sure we need agreement on important decisions. However, I don't think this is major and we already merged feedstocks with that syntax. So I think this ship has sailed.

The issue we have now is if everyone is building each package differently it is more likely some issues creep in or we miss something during a review due to all of the acceptable ways to do something.

I don't think that is an issue. I am OK with 1-2 different forms for script. And, if anyone is found broken, we have to fix it. Right now we know that pip is not.

No one wants broken packages.

Agreed!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure we need agreement on important decisions. However, I don't think this is major...

I think we have different operating definitions of "major". To me, installation is the most important part. The rest is basically metadata. 😉

...and we already merged feedstocks with that syntax. So I think this ship has sailed.

Perhaps it comes across as I'm being too much of a stickler here. Maybe if it were just that syntax, I might not balk so much. Though we have pip installs that don't use --no-deps and here's another. This one matches 4, which can have some surprising effects. We really need a standard like yesterday and we need to stick with it. Also, we can't be allowing things in because we did it before as there are many wrong things we did before that are out in the wild.

Sorry if this comes off as strong, it is merely a reflection of my concern for the current situation with these script lines.

@ocefpaf ocefpaf merged commit 7bd8b41 into conda-forge:master May 4, 2016
@ocefpaf
Copy link
Member

ocefpaf commented May 4, 2016

Thanks @minrk!

@minrk minrk deleted the jupyterhub branch May 4, 2016 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants