Skip to content

fix: get requests-ntlm correct #1828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
beckermr merged 6 commits into from
Jan 9, 2026
Merged

fix: get requests-ntlm correct #1828

beckermr merged 6 commits into from
Jan 9, 2026

Conversation

@beckermr
Copy link
Member

@beckermr beckermr commented Jan 9, 2026 •
edited by h-vetinari
Loading

Follow-up to #1824

@beckermr beckermr requested a review from a team as a code owner January 9, 2026 22:39
@beckermr
Enhance pkg_name validation in feedstock_outputs.py
0eac942 
Update validation to check for non-empty string in pkg_name.
beckermr
beckermr commented Jan 9, 2026
View reviewed changes
@beckermr beckermr marked this pull request as draft January 9, 2026 22:43
beckermr
beckermr commented Jan 9, 2026
View reviewed changes
h-vetinari
h-vetinari reviewed Jan 9, 2026
View reviewed changes
beckermr
beckermr commented Jan 9, 2026
View reviewed changes
@beckermr
Copy link
Member Author

beckermr commented Jan 9, 2026

OK correct error is now raised:

ValueError: Value for 'requests_ntlm' entry must be a list of str (output name, or a glob), but you provided 'r' from 'requests-ntlm'.

beckermr
beckermr commented Jan 9, 2026
View reviewed changes
@beckermr beckermr marked this pull request as ready for review January 9, 2026 22:50
@beckermr beckermr merged commit 3895e9c into main Jan 9, 2026
1 check passed
@beckermr beckermr deleted the beckermr-patch-1 branch January 9, 2026 22:51
h-vetinari
h-vetinari reviewed Jan 9, 2026
View reviewed changes
conda_forge_admin_requests/feedstock_outputs.py
r.raise_for_status()
for pkg_name in pkgs:
if not isinstance(pkg_name, str):
if not isinstance(pkg_name, str) or len(pkg_name) == 1:
Copy link
Member

@h-vetinari h-vetinari Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd split out the error for len=1 TBH; but you provided 'r' from 'requests-ntlm' is not really clear IMO.

@h-vetinari
Copy link
Member

h-vetinari commented Jan 9, 2026

That... was a fast merge 😅

@beckermr
Copy link
Member Author

beckermr commented Jan 9, 2026

Sorry. I was in a rush and needed to move on to other items. I will circle back and address your comments @h-vetinari. Thank you for taking the time to review!

This was referenced Jan 9, 2026
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@h-vetinari h-vetinari h-vetinari left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@beckermr @h-vetinari