Skip to content

sources/azure: consolidate DHCP variants to EphemeralDHCPv4WithReporting#1190

Merged
TheRealFalcon merged 1 commit into
canonical:mainfrom
cjp256:azure-ephemeral-dhcpv4-with-reporting
Jan 18, 2022
Merged

sources/azure: consolidate DHCP variants to EphemeralDHCPv4WithReporting#1190
TheRealFalcon merged 1 commit into
canonical:mainfrom
cjp256:azure-ephemeral-dhcpv4-with-reporting

Conversation

@cjp256
Copy link
Copy Markdown
Contributor

@cjp256 cjp256 commented Jan 14, 2022

  • Update EphemeralDHCPv4WithReporting to subclass EphemeralDHCPv4 for
    consistency (non-functional change).

  • Replace all usage of EphemeralDHCPv4 with EphemeralDHCPv4WithReporting.

  • Converging to one DHCP class exposed an issue with ExitStack patches
    being mixed with decorators. Specifically, it appeared that tests
    that did not enable azure.EphemeralDHCPv4WithReporting mocks had it
    applied anyways from previous tests.

    Presumably ExitStack was overwriting the actual value with the
    mock provided by the decorator? For now, remove some mock patches
    that trigger failures, but future work should move towards a
    consistent approach to prevent undetected effects.

Signed-off-by: Chris Patterson cpatterson@microsoft.com

- Update EphemeralDHCPv4WithReporting to subclass EphemeralDHCPv4 for
  consistency (non-functional change).

- Replace all usage of EphemeralDHCPv4 with EphemeralDHCPv4WithReporting.

- Converging to one DHCP class exposed an issue with ExitStack patches
  being mixed with decorators.  Specifically, it appeared that tests
  that did not enable azure.EphemeralDHCPv4WithReporting mocks had it
  applied anyways from previous tests.

  Presumably ExitStack was overwriting the actual value with the
  mock provided by the decorator?  For now, remove some mock patches
  that trigger failures, but future work should move towards a
  consistent approach to prevent undetected effects.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
Copy link
Copy Markdown
Contributor

@anhvoms anhvoms left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Thanks!

@TheRealFalcon TheRealFalcon merged commit 032a7f2 into canonical:main Jan 18, 2022
@cjp256 cjp256 deleted the azure-ephemeral-dhcpv4-with-reporting branch April 25, 2022 16:06
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