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

User methods 2 #122

Merged
merged 17 commits into from
Nov 24, 2023
Merged

User methods 2 #122

merged 17 commits into from
Nov 24, 2023

Conversation

khalford
Copy link
Member

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 and validate_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 like read and validate.
Docstrings should be very explanative and if they are not please say so!

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.
Copy link
Collaborator

@anish-mudaraddi anish-mudaraddi left a 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

pynetbox_query/pynetboxquery/__main__.py Outdated Show resolved Hide resolved
pynetbox_query/pynetboxquery/utils/parsers.py Show resolved Hide resolved
pynetbox_query/pynetboxquery/utils/read_data.py Outdated Show resolved Hide resolved
Moved argparse functionality to each user script.
Combined ReadData and ReadMethods classes.
Combined ValidateData and ValidateMethods classes.
Copy link
Collaborator

@anish-mudaraddi anish-mudaraddi left a 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

@khalford khalford marked this pull request as ready for review November 23, 2023 16:01
@khalford
Copy link
Member Author

WooHoo!

Copy link
Collaborator

@anish-mudaraddi anish-mudaraddi left a 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-commenter
Copy link

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (db94762) 100.00% compared to head (36569fc) 99.48%.
Report is 36 commits behind head on master.

Files Patch % Lines
...pynetboxquery/user_methods/abstract_user_method.py 80.00% 4 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@dev-0pz dev-0pz left a comment

Choose a reason for hiding this comment

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

LGTM

@khalford khalford merged commit 2820a07 into master Nov 24, 2023
3 checks passed
@khalford khalford deleted the user_methods_2 branch November 24, 2023 09:47
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.

4 participants