Ip checker #28
Merged
Ip checker #28
Conversation
This file contains 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
made code smoother and making sure it worked by adding unit testing which still needs to be finished off
just added or removed every comment
removed pycache files and included a yaml and requirements txt
response to pr changes about docstring and git ignoring files
Co-authored-by: khalford <[email protected]>
added openpyxl to requirements.txt
…into get_dns_records_v2
attempting to ignore .idea
deleted all ignore git ignores meaning that all files that should be ignored are ignored
removed pycache with rm -r--cached fn/__pycache__
removed pycache with rm -r--cached fn/__pycache__ also removed .idea
…into get_dns_records_v3
added ip checker as well as a few unit tests to accompany theses changes the ip checker should check dns and ips for resolvability and print the result to the command line to the user
ip checker that resolves dns and ips and lists out the result with relevant unit tests to accompany it
anish-mudaraddi
requested changes
Jul 30, 2024
| @@ -0,0 +1,36 @@ | |||
| name: Cloud DNS tests | |||
There was a problem hiding this comment.
This file should be placed in a top-level .github/workflows folder
see https://github.com/stfc/SCD-OpenStack-Utils/tree/master/.github/workflows as an example
| :param reverse_order: use reverse or forward order / if true use forward if false use reverse | ||
| :return: the text written into the file | ||
| """ | ||
| print(output_filepath) |
There was a problem hiding this comment.
remove this - we can add logging later
There was a problem hiding this comment.
removed print(output_filepath)
| else: | ||
| unreachable_dns.append(dns_name) | ||
|
|
||
| return reachable_ips, unreachable_ips, reachable_dns, unreachable_dns |
There was a problem hiding this comment.
it feels like you should use a dataclass here - but we can add that later
khalford
requested changes
Aug 5, 2024
|
|
||
| - name: Analyse with pylint | ||
| run: | | ||
| cd reverseforwardrecord && pylint $(git ls-files *.py) |
There was a problem hiding this comment.
Try this and see if it works. It much less messy
Suggested change
| cd reverseforwardrecord && pylint $(git ls-files *.py) | |
| pylint reverseforwardrecord |
resolved changes from reviewers kalibh and anish
anish-mudaraddi
approved these changes
Aug 21, 2024
changing pylint from ls * to pylint reverseforwardrecord
Github was trying to do 20.04 and 22.04 at the same time so nothing started
Was conflicting in terminal, wouldn't let me install python standard library packages
To make pylint happier.
khalford
requested changes
Sep 2, 2024
Went through all errors given as well as refining and changing parts of the code and adding further documentation.
khalford
approved these changes
Sep 13, 2024
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.
added an ip and dns resolver that has its own unit tests which are used to grab the ips from the files after being displayed out in a reverse/ forward order format and then tested on which ips/dns are reachable
the reason it fails in workflow is because it requires an excel file to run or a csv file, and that isnt provided, it also uses argparse