-
Notifications
You must be signed in to change notification settings - Fork 14
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
User methods 2 #122
User methods 2 #122
Conversation
This method will allow a user to query Netbox with a csv and return each device name and if they are in Netbox or not.
Missing tests added. Style changes made.
Read and validate are now in two factory methods that should make future use much easier. Created separatecommands in argparse to make the script more user friendly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good progress - A couple of things namely about favoring inheritance over composition
Moved argparse functionality to each user script. Combined ReadData and ReadMethods classes. Combined ValidateData and ValidateMethods classes.
This should make further user methods easier to introduce.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good - And the latest changes are great - good use of Abstract Classes - you've picked that up really well!
Id recommend renaming the Main
classes so they're the same as the file name
pynetbox_query/pynetboxquery/user_methods/validate_data_fields_in_netbox.py
Show resolved
Hide resolved
WooHoo! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your tests look okay
maybe think about putting your tests in folders - so its the same as the directory structure of your actual code
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #122 +/- ##
===========================================
- Coverage 100.00% 99.48% -0.52%
===========================================
Files 18 34 +16
Lines 446 775 +329
===========================================
+ Hits 446 771 +325
- Misses 0 4 +4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
These changes are to hopefully make the code much more reusable and segmented.
The read factory method allows files to be read in any format so long as an appropriate reading function has been written.
The validate factory method allows any data field in the device dataclass to be check in netbox so long as an appropriate function has been written to do that.
The argparse script now has subcommands that a user to display different help messages and show different command usages.
There are currently two user scripts:
upload_devices_to_netbox.py
andvalidate_data_fields_in_netbox.py
. Both of these scripts should be very simple and only differ by calling each factory method with different arguments.The code is not finished yet and tests are not written/ do not work. Hence why this is a draft.
I have not made any changes to the
netbox_get_id
parts of the project yet as it's a little more complicated but would like to have that method in a sort of factory method likeread
andvalidate
.Docstrings should be very explanative and if they are not please say so!