Closed smierz closed 9 months ago
Totals | |
---|---|
Change from base Build 7435789001: | -3.6% |
Covered Lines: | 371 |
Relevant Lines: | 502 |
thanks for the quick review, I will look into it asap, but probably due to work not before the weekend :/
update:
journal_service
into separate file journal_repository
journal_repository
(test coverage up to 79%)openalex
, journal_service
and journal_repository
re:config
i do have a mild preference for treating the Config object as a singleton that gets instantiated when used or passed explicitly, rather than created as a module-level object - the goal is to have config be single-source and so we want to strongly discourage storing and modifying it.
hmm I think we were both going for a singleton approach, but the difference is the passing of parameters: I was going for imports (=import singleton config from config.py) while you would prefer dependency injection (pass parameters from config singleton in app.py)
Both are valid approaches for handling config propagation in applications and I'm ok with either.
The only concern I have is with creating new Config objects around the application (like crossref.crossref_get
does) because I do not want the config to be mutable from outside the application (= environment variables) during runtime (good luck finding this "bug" if user accidentally changes them).
Anyway, I kicked out the config variable from config.py and switched to dependency injection passing the email parameter through to openalex._openalex_get
Sry for the delay, will merge when I get to keyboard. I think ill deploy this today too
thanks for this! looks great :)
This PR sources homepage_urls for journals from OpenAlex:
crossref
with light changeshomepage_url
" to theJournalBase
objectcrossref.store_papers
because I'm not familiar with SQLAlchemy