Skip to content

Conversation

samoehlert
Copy link
Collaborator

ruff seems to think this is a list and doesn't like how we're string indexing

ruff seems to think this is a list and doesn't like how we're string indexing
Copy link

github-actions bot commented Apr 4, 2025

File Coverage
All files 81%
config/consumers.py 59%
config/urls.py 69%
config/settings/base.py 70%
config/settings/local.py 73%
scram/route_manager/admin.py 85%
scram/route_manager/models.py 70%
scram/route_manager/views.py 81%
scram/route_manager/api/serializers.py 73%
scram/route_manager/api/views.py 83%
scram/templates/403.html 91%
scram/templates/404.html 91%
scram/templates/base.html 99%

Minimum allowed coverage is 50%

Generated by 🐒 cobertura-action against ac2fa9e

@grigorescu
Copy link
Contributor

grigorescu commented Apr 4, 2025

I think this would be better. It still assumes a specific subnet size, which I don't love, but is more flexible.

import ipaddress

first_ip = lambda ip, cidr: str(next(ipaddress.ip_interface(f"{ip}/{cidr}").network.hosts()))

# Take a guess at the gateway IP address. This is a fallback if not otherwise defined.
INTERNAL_IPS += [first_ip(ip, 24) for ip in ips if '.' in ip]
INTERNAL_IPS += [first_ip(ip, 64) for ip in ips if ':' in ip]

@crankynetman
Copy link
Collaborator

I think this would be better. It still assumes a specific subnet size, which I don't love, but is more flexible.

import ipaddress

first_ip = lambda ip, cidr: str(next(ipaddress.ip_interface(f"{ip}/{cidr}").network.hosts()))

# Take a guess at the gateway IP address. This is a fallback if not otherwise defined.
INTERNAL_IPS += [first_ip(ip, 24) for ip in ips if '.' in ip]
INTERNAL_IPS += [first_ip(ip, 64) for ip in ips if ':' in ip]

I think that since it's only used in the local settings, I'd just say let's not touch it and tell ruff to noqa the line and when it breaks we can add this message to the stack of "Seriously, Chris??" messages.

crankynetman
crankynetman previously approved these changes Apr 4, 2025
Copy link
Collaborator

@crankynetman crankynetman left a comment

Choose a reason for hiding this comment

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

Works for me!

also add a comment so we know what the INTERNAL_IPS stuff is about
…thub.com:esnet-security/SCRAM into topic/soehlert/ruff_updates_break_local_settings
Copy link
Collaborator

@crankynetman crankynetman left a comment

Choose a reason for hiding this comment

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

Looks even better. Down with comprehensions!!! 🔱

@samoehlert samoehlert merged commit f67dd2e into main Apr 4, 2025
20 checks passed
@samoehlert samoehlert deleted the topic/soehlert/ruff_updates_break_local_settings branch April 4, 2025 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants