refactor hammer : implement client factory for Compute client #159
Conversation
sudiptob2
force-pushed
the
feat/154/compute-client-factory
branch
2 times, most recently
from
80ce944 to
8c877b9
Compare
Signed-off-by: Sudipto Baral <sudiptobaral.me@gmail.com>
sudiptob2
force-pushed
the
feat/154/compute-client-factory
branch
from
8c877b9 to
15e1b21
Compare
src/gcp_scanner/scanner.py
Outdated
| 'managed_zones': ['name', 'dnsName', 'description', 'nameServers'], | ||
| 'sql_instances': ['name', 'region', 'ipAddresses', 'databaseVersion' | ||
| 'state'], | ||
| 'state'], |
There was a problem hiding this comment.
'state' should be aligned with 'name' (as the first element) from the prior line I think?
There was a problem hiding this comment.
thanks, agreed. I formatted the code with pycharm formatted. Also, the pylint we are using for this project approved this. But the formatting is indeed inconsistent. I will manually fix these formatting errors.
There was a problem hiding this comment.
@under-hill @mshudrak I think there is a bug in the code, a comma, is missing here before state. Thats why pycharm formatted it incorrectly.
| } | ||
|
|
||
| def is_set(config: Optional[dict], config_setting: str) -> Union[dict,bool]: | ||
|
|
There was a problem hiding this comment.
Any specific reason we're adding extra lines here? I'm not up-to-date on the latest style guides so I'm open to new opinions, just curious.
There was a problem hiding this comment.
According to pep8 guidelines, they suggest 2 blank lines. Ref: Pep8 docs, A StackOverflow ans
src/gcp_scanner/scanner.py
Outdated
| if is_set(scan_config, 'static_ips'): | ||
| project_result['static_ips'] = crawl.get_static_ips(project_id, | ||
| compute_client) | ||
| compute_service) |
There was a problem hiding this comment.
This formatting seems odd as many of the others in this file have the args separated into new lines and left-aligned. I'm not too particular on which format we use, but we should be consistent.
|
thanks, agreed. I formatted the code with pycharm formatted. Also, the pylint we are using for this project approved this. But the formatting is indeed inconsistent. I will manually fix these formatting errors. |
|
i am interested to contribute this issue you can please assign me this |
|
We do not assign problem, feel free to just submit PR. |
4 participants
closes #154