[#3818][#4100] Improvement(client-python): Replace all assert in client-python & fix: Validate methods of request in client-python are not correct#4346
Conversation
|
@noidname01 would you please help to review this? |
|
@jingjia88 I think you can also try to solve #4100 in this PR, which shares some problems with this issue. |
|
@jingjia88 would you please fix the conflicts and continue the work as @noidname01 mentioned, thanks. |
|
@jerryshao Sure, I'll work on it. |
|
We're planning to kick off the 0.6.0 release this week, hopes this can be done this week @jingjia88 , thanks a lot. |
jingjia88
changed the title
|
@noidname01 I’ve addressed issue #4100 in this PR as well. Could you please review the modifications? Thank you! |
@jingjia88 Sure, I'll review it ASAP. |
noidname01
left a comment
noidname01
left a comment
There was a problem hiding this comment.
Except for minor issues, LGTM
| assert self.audit_info() is not None, "audit must not be None" | ||
| if self.rest_client is None: | ||
| raise IllegalArgumentException("restClient must be set") | ||
| if not (self.name() and len(self.name().strip()) > 0): |
There was a problem hiding this comment.
I think here can modify into if not self.name() or not self.name.strip()
| raise IllegalArgumentException("name must not be blank") | ||
| if self.type() is None: | ||
| raise IllegalArgumentException("type must not be None") | ||
| if not (self.provider() and len(self.provider().strip()) > 0): |
|
@noidname01 can you please take another review when you have time, thanks! Generally the PR looks good to me. |
…nt-python & fix: Validate methods of request in client-python are not correct (#4346) ### What changes were proposed in this pull request? - Fix `validate` errors (attributes are not allowed to be None) in `catalog_update_request.py` . - Unify the implementation way of `validate` : using raise. - Replace all `assert` with defined exception. ### Why are the changes needed? Fix: #3818 Fix: #4100 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? test with` ./gradlew client:client-python:test`
3 participants
What changes were proposed in this pull request?
validateerrors (attributes are not allowed to be None) incatalog_update_request.py.validate: using raise.assertwith defined exception.Why are the changes needed?
Fix: #3818, #4100
Does this PR introduce any user-facing change?
No.
How was this patch tested?
test with
./gradlew client:client-python:test