Assignment from no return #658
Conversation
… lines in run_tests.py, exception thrown by 'git remote add origin' when the remote already exists will not exit Signed-off-by: Mark Cohen <markcoh@amazon.com>
Signed-off-by: Mark Cohen <markcoh@amazon.com>
macohen
requested review from
VachaShah,
dblock,
harshavamsi,
axeoman,
Shephalimittal,
saimedhi and
florianvazelle
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #658 +/- ##
==========================================
+ Coverage 72.13% 72.14% +0.01%
==========================================
Files 89 89
Lines 7945 7945
==========================================
+ Hits 5731 5732 +1
+ Misses 2214 2213 -1 ☔ View full report in Codecov by Sentry. |
updated CHANGELOG.md Signed-off-by: Mark Cohen <markcoh@amazon.com>
macohen
force-pushed
the
assignment-from-no-return
branch
from
96da2f1
to
07c11c5
Compare
added assert to test get_value_filter returning None Signed-off-by: Mark Cohen <markcoh@amazon.com>
macohen
force-pushed
the
assignment-from-no-return
branch
from
f0292f6
to
0528052
Compare
|
Few tests are failing. Please try to make them pass. |
returning None from test_faceted_search.Facet.get_value_filter Signed-off-by: Mark Cohen <markcoh@amazon.com>
…renced them Signed-off-by: Mark Cohen <markcoh@amazon.com>
CHANGELOG.md
Outdated
| @@ -3,6 +3,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) | |||
|
|
|||
| ## [Unreleased] | |||
| ### Added | |||
| - Added pylint `assignment-from-no-return` (([#657](https://github.com/opensearch-project/opensearch-py/pull/657)) | |||
There was a problem hiding this comment.
Please change PR number 658
| ) | ||
| else: | ||
| print(e) | ||
| sys.exit(1) |
There was a problem hiding this comment.
should this sys.exit(1) be moved into else?
There was a problem hiding this comment.
My intent here was to make it clear why the exit was happening. I didn't want to change the previous behavior especially after I found the TEST_OPENSEARCH_NOFETCH variable.
There was a problem hiding this comment.
@macohen, I apologize if I am causing any confusion, but I believe there's a change in the behavior for e.returncode == remote_origin_already_exists.
It seems that there shouldn't be a sys.exit(1) when e.returncode == remote_origin_already_exists. Please correct me if I'm mistaken.
There was a problem hiding this comment.
Sorry for the poor explanation on my end. Prior to my change, when the git repository already had a remote origin added, the whole process exited. I was considering just ignoring that state, but then found the TEST_OPENSEARCH_NOFETCH variable which can be set when you want that state to be ignored. I figured it would be better to maintain the behavior of the script in that case and also tell the user that TEST_OPENSEARCH_NOFETCH could help if exiting was not desired. I hope that helps...
Signed-off-by: Mark Cohen <markcoh@amazon.com>
macohen
force-pushed
the
assignment-from-no-return
branch
from
e23376f
to
c2c9c22
Compare
Description
* updated to pass this by marking Facet.get_value_filter(self, filter_value: Any) -> Any as an abc.abstractmethod* implemented these missing methods in two subclasses to return NoneIssues Resolved
progress on #610
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.