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

Moved migration script in from core. #1071

Merged
merged 5 commits into from Oct 16, 2018

Conversation

leonardr
Copy link
Contributor

Circulation portion of NYPL-Simplified/server_core#974. Updated submodule and moved in a migration script. The only change I made to the migration script was changing the code that modifies sys.path so that 'from core.model import' referred to the right place.

@leonardr leonardr requested a review from aslagle October 16, 2018 18:25
@leonardr
Copy link
Contributor Author

Not sure what's going on here but it's something about the Travis environment, not anything to do with this code.

Here's a branch that doesn't change anything and its integration tests fail the same way: #1072

@leonardr
Copy link
Contributor Author

A new version of urllib3 was just released and I think one of its changes, probably urllib3/urllib3#1409, caused this problem. I don't know whether this needs to be fixed in urllib3, requests, or if we need to do something, but for now, refusing to use the new release seems to fix the problem.

requirements.txt Outdated
@@ -5,6 +5,7 @@ elasticsearch-dsl<2.0.0
pillow
psycopg2
requests==2.18.4
urllib3!=1.24
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd do < 1.24 since there may be more releases with the same problem.

medium = cls.rbdigital_medium_to_simplified_medium.get(
rbdigital_medium, Edition.BOOK_MEDIUM
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this code out of the if include_bibliographic block so it would always run and we would never have a situation where it was unclear what medium a title was. The medium is bibliographic information, but the RBdigital code also uses it to determine format information.

We could probably get rid of the include_bibliographic thing altogether, but there's no rush.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked the other license sources and didn't find any other cases where we relied on the default Metadata.medium behavior that was removed from core.

@leonardr leonardr merged commit 330ff32 into master Oct 16, 2018
@leonardr leonardr deleted the prevent-audiobooks-from-being-treated-as-books branch October 16, 2018 21:09
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.

None yet

2 participants