Skip to content

Conversation

@NyanHelsing
Copy link
Contributor

Purpose

Running docker-compose up mfr_requirements fails because it waits for user interaction. Removing -e lets pip install aiohttppretty without failing because pip doesn't know what to do.

Changes

Remove the -e flag from the requirements file

Side effects

This flag looks like its been here since the dependency was first added, but there is no reason stated. There may be a reason its there? If so we can look at --ignore-installed as an alternative

QA Notes

Probably no.

Deployment Notes

Probably no. This only affects dev environments.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 72.448% when pulling 6ba2bce on birdbrained:bf/fix-req-install into f943de8 on CenterForOpenScience:develop.

@coveralls
Copy link

coveralls commented Feb 7, 2018

Coverage Status

Coverage remained the same at 72.448% when pulling 8c3f9c6 on birdbrained:bf/fix-req-install into f943de8 on CenterForOpenScience:develop.

Copy link
Member

@felliott felliott left a comment

Choose a reason for hiding this comment

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

The change is GTG, but please git commit --amend the commit message to follow the guidelines in https://chris.beams.io/posts/git-commit/ (The 50 char max in the subject isn't a hard requirements, but try to keep it <60 chars).

It makes it neccesary to interact with the container, so running
`docker-compose up mfr_requirements works properly`
@NyanHelsing
Copy link
Contributor Author

My bad, I thought my newline was putting it in the body not the subject. (Normally I don't use commit -m but an editor to write commit messages. Serves me right for trying something new.)

@felliott
Copy link
Member

felliott commented Feb 8, 2018

LGTM! Merging.

Copy link
Contributor

@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.

@felliott and @birdbrained

Removing -e mode on WB (and on MFR) causes improper installation of aiohttpretty. MFR passes because aiohttpretty is not used at all. When I try to import the package in the test, it fails as expected.

See comments here: CenterForOpenScience/waterbutler#321 (review)

Update: tests are fixed after upgrading aiohttpretty to 0.1.0.

See comments here: CenterForOpenScience/waterbutler#321 (review)

  • TODO: we should remove or at least upgrade aiohttpretty for MFR

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