This repository was archived by the owner on Jun 10, 2024. It is now read-only.
Refactor utils connect db #769
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Refactored one of db connections. more to follow.
binux
reviewed
Mar 1, 2018
pyspider/database/__init__.py
Outdated
| return ResultDB(path) | ||
| else: | ||
| raise LookupError | ||
| return _connect_sqlite(parsed,dbtype) | ||
| elif engine == 'mongodb': | ||
| url = url.replace(parsed.scheme, 'mongodb') |
There was a problem hiding this comment.
The url replacement logic should be a part of _connect_mongodb.
binux
reviewed
Mar 1, 2018
pyspider/database/__init__.py
Outdated
| from .mongodb.resultdb import ResultDB | ||
| return ResultDB(url, **parames) | ||
| else: | ||
| raise LookupError | ||
| elif engine == 'sqlalchemy': | ||
| if not other_scheme: |
binux
reviewed
Mar 1, 2018
pyspider/database/__init__.py
Outdated
| @@ -154,23 +92,104 @@ def _connect_database(url): # NOQA | |||
| raise LookupError('not supported dbtype: %s', dbtype) | |||
| elif engine == 'elasticsearch' or engine == 'es': | |||
| # in python 2.6 url like "http://host/?query", query will not been splitted | |||
There was a problem hiding this comment.
This comment is for L157, should be moved as well.
binux
reviewed
Mar 1, 2018
pyspider/libs/utils.py
Outdated
| elif days < 334: # 11mo, since confusing for same month last year | ||
| format = "%(month)s-%(day)s" if shorter else \ | ||
| "%(month)s-%(day)s at %(time)s" | ||
| ret_, ret_format = fix_full_format(days, seconds, relative, shorter, local_date, local_yesterday) |
There was a problem hiding this comment.
The refectory here is not correct. The original code is to generate a format, it has to be filled with actual date information in L140, it shouldn't be returned directly.
There was a problem hiding this comment.
I'm not sure what's incorrect compared to the original code. The original code has three "direct" return calls after the decision point "if relative and days == 0:" at line 109 of the original code. Which are the three cases we return in the refactoring.
binux
suggested changes
Mar 1, 2018
binux
approved these changes
Mar 7, 2018
brian6peng
pushed a commit
to brian6peng/pyspider
that referenced
this pull request
Jun 20, 2018
* part 1 * [Doc] Update coverage links for the fork * Refactor. Changes to utils and _connect_db * Refactored in utils Refactored one of db connections. more to follow. * Refactor. Split up some more connects * Rm. Removed unused textfile * Doc. rewrote README.md to original * Refactor. Wrong variable name fix. * Refactor, applied suggested changes in the review in the database connector. * Refactor, changed name of return variable gotten from fix_full_format in format_date
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We a group of students have been looking at coverage and cyclomatic complexity on this repository and as an extra part of our assignment we're looking to get our refactor accepted. This pull request lowers the cyclomatic complexity of the functions _connect_database@49-176@./pyspider/database/init.py and format_date@72-147@./pyspider/libs/utils.py, By splitting the functions up at logical points.