clean-up (linting) customising scripts #4046
Conversation
…firewall-orchestrator into feat/v9/app-data-import-customizing
…com/tpurschke/firewall-orchestrator into feat/v9/app-data-import-customizing
tpurschke
changed the title
…firewall-orchestrator into feat/v9/app-data-import-customizing
tpurschke
changed the title
|
ErikPre
left a comment
ErikPre
left a comment
There was a problem hiding this comment.
Just some minor comments to go through, other than that looks very good!
|
|
||
| # check for changes from product-specific FW API, if we are importing from file we assume config changes | ||
| # TODO: implement real change detection | ||
| # open issue: implement real change detection |
There was a problem hiding this comment.
If doing this, maybe we add practice here to add the issue number
| raise ApiLoginFailed("RLM api: ERROR: did not receive an OAUTH token during login" + \ | ||
| ", api_url: " + str(api_url) + \ | ||
| ", status code: " + str(response)) | ||
| raise ApiFailureError("api: error during login to url: " + str(api_url) + " with user " + user) from None |
There was a problem hiding this comment.
use f-string here or parameterized string here
| if response.status_code == HTTP_OK: | ||
| return json.loads(response.text)["access_token"] | ||
| raise ApiLoginFailedError( | ||
| "RLM api: ERROR: did not receive an OAUTH token during login" |
There was a problem hiding this comment.
use f-string here or parameterized string here
| except requests.exceptions.RequestException: | ||
| raise ApiServiceUnavailable ("api: error while getting owners from url: " + str(api_url) + " with token " + token) from None | ||
| raise ApiServiceUnavailableError( | ||
| "api: error while getting owners from url: " + str(api_url) + " with token " + token |
There was a problem hiding this comment.
use f-string here or parameterized string here
| ", api_url: " + str(api_url) + \ | ||
| ", status code: " + str(response)) | ||
| raise ApiFailureError( | ||
| "api: ERROR: could not get owners, api_url: " + str(api_url) + ", status code: " + str(response) |
There was a problem hiding this comment.
use f-string here or parameterized string here
| from scripts.customizing.fwo_custom_lib.basic_helpers import get_logger, read_custom_config | ||
|
|
||
| default_config_filename: str = "/usr/local/fworch/etc/secrets/customizingConfig.json" | ||
| ipam_git_repo_target_dir: str = "/usr/local/fworch/etc/ipamRepo" |
There was a problem hiding this comment.
I suggest using this as a constant
| from requests import Session, exceptions | ||
|
|
||
| default_api_url = 'https://localhost:8888/api/' | ||
| default_api_url: str = "https://localhost:8888/api/" |
There was a problem hiding this comment.
I suggest using this as a constant
| self.request_body_template: dict[str, Any] = self._build_request_body_template() | ||
|
|
||
| def _init_placeholders(self) -> None: | ||
| self.org_id_placeholder: str = "{orgid}" |
There was a problem hiding this comment.
I suggest using this as a constant
| base_dir_etc = fwo_base_dir + "etc/" | ||
| cmdb_repo_target_dir = fwo_tmp_dir + "cmdb-repo" | ||
| default_config_file_name = base_dir_etc + "customizingConfig.json" | ||
| fwo_base_dir: str = "/usr/local/fworch/" |
There was a problem hiding this comment.
I suggest using this as a constant
| default_config_filename: str = "/usr/local/fworch/etc/secrets/customizingConfig.json" | ||
| ipam_git_repo_target_dir: str = "/usr/local/fworch/etc/ipamRepo" | ||
| SUBNET_NAME_PARTS_MIN_COUNT: int = 3 | ||
| git_any: Any = git |
There was a problem hiding this comment.
I see why this was done, but binding a module to a variable i not recommended. Here ignoring pylance is more than valid.
2 participants
also preparing N001 app-data-import