Handle ModelNotFoundException in Cropty validation - #435
Closed
manav-py wants to merge 1 commit into
Closed
Conversation
Owner
|
@mnv-cs You were spot on here as well! However, the approach in #436 turned out to be a bit cleaner. Another blocker was that the PR author and the commit author didn't match, which goes against our repository's contribution guidelines. We really appreciate your time and effort on this though! You're always welcome to contribute again, whether that's adding new modules or fixing bugs. We'd be happy to review and merge your future PRs, just make sure your GitHub account matches the commit author next time. |
Author
|
Thanks for the review and for the detailed feedback! I appreciate you
taking the time to explain both the implementation preference and the
commit author issue. I've fixed my Git configuration so future commits will
use the correct author information. Looking forward to contributing again!
…On Fri, 24 Jul 2026 at 09:40, Kaif ***@***.***> wrote:
*kaifcodec* left a comment (kaifcodec/user-scanner#435)
<#435 (comment)>
@mnv-cs <https://github.com/mnv-cs> You were spot on here as well!
However, the approach in #436
<#436> turned out to be a
bit cleaner. Another blocker was that the PR author and the commit author
didn't match, which goes against our repository's contribution guidelines.
We really appreciate your time and effort on this though! You're always
welcome to contribute again, whether that's adding new modules or fixing
bugs. We'd be happy to review and merge your future PRs, just make sure
your GitHub account matches the commit author next time.
—
Reply to this email directly, view it on GitHub
<#435?email_source=notifications&email_token=B2ROOJPY6SP7Q73BIDNH2YD5GLORRA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBWGYYDKMBRGAYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5066050100>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/B2ROOJMM52DDVUBE56AYFDL5GLORRAVCNFSNUABGKJSXA33TNF2G64TZHMYTANZZGA4DSMJYGY5US43TOVSTWNBZGU4DGOBRGYYDLILWAI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Owner
|
No worries @mnv-cs ! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR updates the Cropty validation logic to inspect the response body for
ModelNotFoundExceptionbefore returning an unexpected status error.What changed
200and404responses.ModelNotFoundExceptionin the response body for all other HTTP status codes.Result.available()when the API indicates the user does not exist, even if the response status is not explicitly handled.Why
Previously, only
200and404responses were handled explicitly. Any other status code (such as403,429, or500) would immediately return:without inspecting the response body.
With this change, if the API returns a
ModelNotFoundExceptionin the response body for an unexpected status code, it is correctly treated as an available username instead of an unexpected error.