-
Notifications
You must be signed in to change notification settings - Fork 97
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
Improve visualization branch + Feature comparing #50
base: master
Are you sure you want to change the base?
Conversation
Bring Python to 3.9 in Travis. Thanks!
Boto3 update 1.16.24
-replaced run_raw call with file reference in wrapper
-moved resources execution to apply_filter -added command line option for resource filters
…run without credentials, as it will run very slowly.
-renamed ListingFile -> FilteredListing -moved non-default filters to fixing_filter.py
- added print-html from old branch - added count number
- moved most html-printing functionality to new py-file
- fixed html-file call from directory
- Combine title and searchbox in header - Move timestamps to footer
-Remove stray comments -Remove convert_unfilterList -Add error-hadling to JSON-dependent resources-block -Adjust resources-dependent query functions
-Finish display of comparison in HTML -Add display of resource IDs for found resources -Change acquire_listing return value to ResultListing object -Refactor do_list_files into info summary and resource ID search
-Fix comparing ResultListing-functions -Add JSON-functions to FIlteredListing -Save dir + unfilter in JSON-files -Delete stray comments
This list of patches splits out the raw listing from the filtered listings, and should allow better handling of different kinds of errors in the long term. In the short term, it adds the possibility to "unfilter" the automatically applied filters that until now were baked in.
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.
Thanks, looking almost ready!
aws_list_all/__main__.py
Outdated
viewhtml.add_argument('-p', '--parallel', default=32, type=int, help='Number of request to do in parallel') | ||
viewhtml.add_argument('-d', '--directory', default='.', help='Directory to save result listings to') | ||
viewhtml.add_argument('-v', '--verbose', action='count', help='Print detailed info during run') | ||
viewhtml.add_argument('-c', '--profile', help='Use a specific .aws/credentials profile.') |
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.
Shouldn't viewhtml be more similar to "show"? This looks like it's doing all the querying again.
aws_list_all/__main__.py
Outdated
|
||
new = 2 | ||
url = os.getcwd() + "/compare.html" | ||
webbrowser.open(url,new=new) |
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.
This section should go into its own module
aws_list_all/generate_html.py
Outdated
print(' margin-bottom: 12px;\n') | ||
print('}\n') | ||
print('</style>\n') | ||
print('</head>\n') |
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.
The static stuff should probably go into a CONSTANT with triple quotes """, then it can be multiline?
aws_list_all/generate_html.py
Outdated
print(' </td>') | ||
print(' </tr>\n') | ||
print('</table>') | ||
print('</div>') |
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.
I am tempted to suggest a more generic solution here, but this is actually quite readable.
if listing.service == 'elasticache' and listing.operation == 'DescribeCacheSubnetGroups': | ||
response['CacheSubnetGroups'] = [ | ||
g for g in response.get('CacheSubnetGroups', []) if g.get('CacheSubnetGroupName') != 'default' | ||
] |
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.
This branch probably needs a rebase to the current master since I merged the other PR
-Move access to html-file from main to generate_html -Refactor sections from html-head into string constants
-Delete old print-html command -Set html as option to 'view' -Implement view's function to execute query+show or print html results
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 to me, modulo the issues raised. I'd probably merge and fix up as is, but feel free to update the PR!
os.chdir(args.directory) | ||
increase_limit_nofiles() | ||
services = args.service or get_services() | ||
do_consecutive( |
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.
I think this could be expressed as a sequence of query & view.
aws_list_all/__main__.py
Outdated
# Combine the features of query and show and optionally print the findings | ||
# from query to an HTML file to be viewed in a browser | ||
view = subparsers.add_parser( | ||
'view', description='Print a listing to an HTML file to be viewed in a browser', |
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.
This should probably be clearer about that it is actually querying the data.
"""Open a file to write HTML-content in and return the original system output and file path""" | ||
origout = sys.stdout | ||
f = open(name, 'w') | ||
sys.stdout = f |
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.
I really don't like bending sys.stdout here like this. Ideally, all these functions should just add a string together (lines = []; lines.append instead of print; return "\n".join(lines))
def diffInListing(cls, base, diff): | ||
return cls(base.input, base.result_type, base.details, base.id_list, diff) | ||
|
||
def __eq__(self, other): |
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.
Here you can just define eq and lt and then use https://docs.python.org/3/library/functools.html#functools.total_ordering to generate the rest
-Change printing of html-strings to returning string at the end -Write to html-file after finished joining all strings at the end -Adjust view-command description and help
OK, looks good for now. There are still some merge conflicts now? |
No description provided.