Skip to content
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.

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

T6998: dhcp.py fix datetime to be zone aware, add fix for hostname #4277

Open
wants to merge 2 commits into
base: current
Choose a base branch
from

Conversation

metron2
Copy link

@metron2 metron2 commented Jan 5, 2025

Change summary

This fixes show dhcp server leases to work again.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T6998

Related PR(s)

How to test / Smoketest result

I ran "show dhcp server leases" and it didn't work

show dhcp server statistics 
Traceback (most recent call last):
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 545, in <module>
    res = vyos.opmode.run(sys.modules[__name__])
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/opmode.py", line 312, in run
    res = func(**args)
          ^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 335, in _wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 360, in show_pool_statistics
    pool_data = _get_raw_pool_statistics(family=family, pool=pool)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 243, in _get_raw_pool_statistics
    leases = len(_get_raw_server_leases(family=family, pool=p))
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/dhcp.py", line 135, in _get_raw_server_leases
    data_lease['remaining'] = lease['expire_timestamp'] - datetime.now(timezone.utc)
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
TypeError: can't subtract offset-naive and offset-aware datetimes

fixing that bug led to finding the bug with the hostname length.

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Copy link

github-actions bot commented Jan 5, 2025

👍
No issues in PR Title / Commit Title

@metron2 metron2 changed the title T6998 dhcp.py - fix datetime to be zone aware, add fix for hostname T6998: dhcp.py fix datetime to be zone aware, add fix for hostname Jan 5, 2025
@c-po
Copy link
Member

c-po commented Jan 5, 2025

@metron2 thanks for the fixes. Could you please pack them into two individual commits as they solve two different issues?

@metron2
Copy link
Author

metron2 commented Jan 6, 2025

@metron2 thanks for the fixes. Could you please pack them into two individual commits as they solve two different issues?

Sure happy to reorganize them. Any thoughts re dateformats for users? On the dev portal it was brought up that this modifies the presentation from a custom format %Y/%m/%d %H:%M:%S to ISO.

On my system, I needed both of these fixes to get "show dhcp server leases" working again.

src/op_mode/dhcp.py Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Jan 7, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link

github-actions bot commented Jan 8, 2025

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the conflicts label Jan 8, 2025
Copy link

github-actions bot commented Jan 8, 2025

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests (no interfaces) ❌ failed
  • CLI Smoketests (interfaces only) ❌ failed
  • Config tests ❌ failed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants