Skip to content

Conversation

@cslzchen
Copy link
Contributor

@cslzchen cslzchen commented Nov 21, 2017

Ticket

MFR Ticket: https://openscience.atlassian.net/browse/SVCS-140
Replaces: #248

WB Ticket: https://openscience.atlassian.net/browse/SVCS-139
WB PR: CenterForOpenScience/waterbutler#302

Purpose

All credits go to @Johnetordoff 🎆 , I just rebased/squashed the commits and fixed conflicts.

Let MFR run later versions of setuptools (>= 31.0 setuptools). Currently, the latest version is 37.0.0.

Changes

Creates version.py file where __version__ is defined to prevent circular import.

Side Effects

How can other applications, say Waterbutler, refer to this version? Please see my comment in the code: #301 (review)

QA Notes

No

Developer Notes

After testing this PR, you need to downgrade your setuptools to work with current develop based branch. (Use your own virtualenv directory instead)

pip uninstall -y setuptools
rm -f ~/.virtualenv/mfr/lib/python3.5/site-packages/mfr-nspkg.pth
pip install setuptools==30.4.0

Deployment Notes

Please review to see if this affects you.

@coveralls
Copy link

coveralls commented Nov 21, 2017

Coverage Status

Coverage decreased (-0.02%) to 67.998% when pulling 50e6303 on cslzchen:feature/fix-setup-tools into 8bb2dd4 on CenterForOpenScience:develop.

@cslzchen cslzchen force-pushed the feature/fix-setup-tools branch from 50e6303 to f1f9e39 Compare November 21, 2017 19:58
 move `__version__` from `./mfr/__init__.py`
 to `./version.py`, update imports and
 a few minor improvements.
@cslzchen cslzchen force-pushed the feature/fix-setup-tools branch from f1f9e39 to 85cee26 Compare November 21, 2017 20:01
@cslzchen cslzchen changed the title [WIP] [SVCS-140] Bump setuptools to 37.0.0 (latest) [SVCS-140] Bump setuptools to 37.0.0 (latest) Nov 21, 2017
keen_payload = copy.deepcopy(metrics)
keen_payload['meta'] = {
'mfr_version': mfr.__version__,
# 'wb_version': waterbutler.__version__,
Copy link
Contributor Author

@cslzchen cslzchen Nov 21, 2017

Choose a reason for hiding this comment

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

I remove this line because I am not sure how this would work in the new code. #SideEffect

Copy link
Member

Choose a reason for hiding this comment

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

As we discussed with Fitz, there may be ways to get the waterbutler version. If putting the version in version.py leaves the version unavailable for import we could try putting it in waterbutler/settings.py .

mako==1.0.1
raven==5.27.0
setuptools==30.4.0
setuptools==37.0.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ticket was targeting >=31.0. I tried the latest version and nothing breaks. The doc is also updated.

@coveralls
Copy link

coveralls commented Nov 21, 2017

Coverage Status

Coverage decreased (-0.02%) to 67.998% when pulling 85cee26 on cslzchen:feature/fix-setup-tools into 8bb2dd4 on CenterForOpenScience:develop.

Copy link
Member

@TomBaxter TomBaxter left a comment

Choose a reason for hiding this comment

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

I have reproduced the issue and tested that his fixes the issue. Looks good to me.

keen_payload = copy.deepcopy(metrics)
keen_payload['meta'] = {
'mfr_version': mfr.__version__,
# 'wb_version': waterbutler.__version__,
Copy link
Member

Choose a reason for hiding this comment

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

As we discussed with Fitz, there may be ways to get the waterbutler version. If putting the version in version.py leaves the version unavailable for import we could try putting it in waterbutler/settings.py .

Copy link
Contributor Author

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

Move to PCR 🎆 🎆 . (I cannot approve my own PR)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants