Bug fixes and improvments#273
Merged
Merged
Conversation
…egitimate falsy values (like `0` or `False`) because they check `elif value:` instead of `elif value is not None:`. Fix:** Update the conditionals to explicitly check for `None`.
… use sendall instead of send to make sure all the data is sent.
The `get_format_32()` function is unnecessary as `struct.calcsize(">I")` is guaranteed to be 4 bytes in Python.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR focuses on improving EPP protocol robustness and fixing XML template issues, along with a version bump to reflect the changes.
Changes:
- Simplifies the 32-bit length-field packing format and improves socket send/receive behavior.
- Extends the domain create template to support multiple DNSSEC DS records and fixes an invalid XML closing tag.
- Adjusts escaping to preserve falsy-but-valid values in nested structures; bumps package version and updates
.gitignore.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pyepp/epp.py | Simplifies length packing, improves _read() buffering logic, switches _write() to sendall(). |
| pyepp/command_templates.py | Adds support for list/single DNSSEC DS records; fixes domain:pw closing tag typo. |
| pyepp/base_command.py | Preserves non-None falsy values during nested escaping. |
| pyepp/init.py | Bumps package version to 0.2.0 (used by dynamic versioning). |
| .gitignore | Adds ignores for GEMINI/CLAUDE markdown files. |
Comments suppressed due to low confidence (1)
pyepp/base_command.py:91
- The change from
elif value:toelif value is not None:is meant to preserve falsy but valid values (e.g.,0,False) in nested lists/dicts. Current unit tests cover escaping of strings but do not assert the new behavior for falsy values, so regressions would be easy to miss. Add test cases for0/Falsein both__escape_listand__escape_dict.
result[key] = self.__escape_dict(value)
elif isinstance(value, str):
result[key] = escape(value)
elif value is not None:
result[key] = value
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…o use value is not None instead of a simple truthiness check (if value). This ensures that 0 and False are not filtered out of the parameters passed to the XML template.
…pp into bug-fixes-and-improvments
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.
This pull request introduces several improvements and bug fixes to the PyEPP package, focusing on better handling of falsy values in command preparation, enhanced DNSSEC support in domain creation, improved socket communication, and expanded test coverage. The changes also include updates to development dependencies and minor template corrections.
Key highlights:
0andFalse), ensuring accurate XML rendering and data transmission.Command Preparation and Data Handling
BaseCommand._prepare_command,__escape_list, and__escape_dictto preserve falsy values (0,False) instead of filtering them out, ensuring these values are correctly rendered in XML commands. [1] [2] [3]test_prepare_command_falsy_values,test_escape_list,test_escape_dict). [1] [2]DNSSEC and Domain Creation Enhancements
EPP Communicator and Socket Robustness
_read) and write (_write) methods to handle empty reads, partial data, and usesendallfor reliability. The_readmethod now returnsNoneon empty or broken responses, improving error handling. [1] [2] [3]get_format_32to always return">I", removing unnecessary logic.EppCommunicator, covering error scenarios, connection logic, and dry-run behavior.Template and Dependency Fixes
<domain:pw>closing tag).requirements.dev.txtto newer versions.Miscellaneous Improvements
0.2.0.This pull request introduces several improvements and bug fixes to the PyEPP package, focusing on better handling of falsy values in command preparation, improved DNSSEC record rendering, more robust socket communication, and general code quality enhancements. The changes also include new and updated tests to ensure correctness and reliability.Core logic and bug fixes:
_prepare_command,__escape_list, and__escape_dictmethods inBaseCommandto preserve falsy values like0andFalse, instead of filtering them out, ensuring these values are correctly rendered in XML commands. [1] [2] [3]command_templates.pyso that both single and multiple DS records are rendered correctly in the generated XML.</domains:pw>to</domain:pw>.Socket communication improvements:
get_format_32function to always return the correct struct format, and improved the_readand_writemethods inEppCommunicatorto usesendallfor reliability and to handle partial reads and empty chunks more gracefully. [1] [2] [3] [4]Testing and code quality:
test_base_command.pyandtest_domain.pyto verify preservation of falsy values and correct DNSSEC rendering, and improved socket communication tests intest_pyepp.py. [1] [2] [3] [4]requirements.dev.txtfor better code quality and security.0.2.0in__init__.py.