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

Address deploy-agent lint errors #1642

Merged
merged 3 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ repos:
hooks:
- id: ruff
args: [ --fix ]
- id: ruff-format
- repo: https://github.com/djlint/djLint
rev: v1.34.1
hooks:
- id: djlint-reformat-django
- id: djlint-django
args: [--ignore="H031"]
6 changes: 3 additions & 3 deletions deploy-agent/deployd/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def serve_build(self) -> None:
# for each service, we check the container health status
log.info(f"the current service is: {status.report.envName}")
try:
if status.report.redeploy == None:
if status.report.redeploy is None:
status.report.redeploy = 0
healthStatus = get_container_health_info(status.build_info.build_commit, status.report.envName, status.report.redeploy)
if healthStatus:
Expand All @@ -145,7 +145,7 @@ def serve_build(self) -> None:
status.report.containerHealthStatus = healthStatus
status.report.state = None
if "unhealthy" not in healthStatus:
if status.report.wait == None or status.report.wait > 5:
if status.report.wait is None or status.report.wait > 5:
status.report.redeploy = 0
status.report.wait = 0
else:
Expand Down Expand Up @@ -228,7 +228,7 @@ def serve_forever(self) -> None:
while True:
try:
self.serve_build()
except:
except Exception:
log.exception("Deploy Agent got exception: {}".format(traceback.format_exc()))
finally:
time.sleep(self._config.get_daemon_sleep_time())
Expand Down
10 changes: 5 additions & 5 deletions deploy-agent/deployd/common/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down Expand Up @@ -52,11 +52,11 @@ class DeployType(object):
RESTART = 'RESTART'
STOP = 'STOP'

class DeployErrorSource(object):
class DeployErrorSource(object):
TELEFIG = 'TELEFIG'
UNKNOWN = 'UNKNOWN'

class DeployError(object):
class DeployError(object):
TELEFIG_UNAVAILABLE = 'TELEFIG_UNAVAILABLE'
UNKNOWN = 'UNKNOWN'

Expand Down Expand Up @@ -197,7 +197,7 @@ def load_from_json(self, json_value) -> None:
def to_json(self) -> dict:
json = {}
for key, value in self.__dict__.items():
if type(value) is dict:
if isinstance(value, dict):
json[key] = {k: v for k, v in value.items()}
elif value:
if hasattr(value, "__dict__"):
Expand Down
26 changes: 13 additions & 13 deletions deploy-agent/deployd/common/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def get_info_from_facter(keys) -> Optional[dict]:
else:
log.warn("Got empty output from facter by keys {}".format(keys))
return None
except:
except Exception:
log.exception("Failed to get info from facter by keys {}".format(keys))
return None

Expand All @@ -231,17 +231,17 @@ def redeploy_check_for_container(labels, service, redeploy) -> int:
if redeploy < max_retry:
return redeploy + 1
return 0

def redeploy_check_for_non_container(commit, service, redeploy, healthcheckConfigs):
if healthcheckConfigs.get("HEALTHCHECK_REDEPLOY_WHEN_UNHEALTHY") == "True":
log.info(f"Auto redeployment is enabled on service {service}")
max_retry = int(healthcheckConfigs.get("HEALTHCHECK_REDEPLOY_MAX_RETRY", REDEPLOY_MAX_RETRY))
if redeploy < max_retry:
log.info(f"redeploy is {redeploy}; max retry is {max_retry}")
create_sc_increment(name='deployd.service_health_status',
create_sc_increment(name='deployd.service_health_status',
tags={"status": "redeploy", "service": service, "commit": commit})
return "redeploy-" + str(redeploy + 1)
create_sc_increment(name='deployd.service_health_status',
create_sc_increment(name='deployd.service_health_status',
tags={"status": "unhealthy", "service": service, "commit": commit})
return service + ":unhealthy"

Expand All @@ -261,13 +261,13 @@ def redeploy_check_without_container_status(commit, service, redeploy):
try:
resp = requests.get(url)
if resp.status_code >= 200 and resp.status_code < 300:
create_sc_increment(name='deployd.service_health_status',
create_sc_increment(name='deployd.service_health_status',
tags={"status": "healthy", "service": service, "commit": commit})
return service + ":healthy"
except requests.ConnectionError:
return redeploy_check_for_non_container(commit, service, redeploy, healthcheckConfigs)
return redeploy_check_for_non_container(commit, service, redeploy, healthcheckConfigs)

def get_container_health_info(commit, service, redeploy) -> Optional[str]:
try:
log.info(f"Get health info for service {service} with commit {commit}")
Expand All @@ -289,39 +289,39 @@ def get_container_health_info(commit, service, redeploy) -> Optional[str]:
labels = parts[2].split(',')
ret = redeploy_check_for_container(labels, service, redeploy)
if ret > 0:
create_sc_increment(name='deployd.service_health_status',
create_sc_increment(name='deployd.service_health_status',
tags={"status": "redeploy", "service": service, "commit": commit})
return "redeploy-" + str(ret)
result.append(f"{name}:{status}")
except:
except Exception:
continue
returnValue = ";".join(result) if result else None
if returnValue and "unhealthy" in returnValue:
create_sc_increment(name='deployd.service_health_status',
create_sc_increment(name='deployd.service_health_status',
tags={"status": "unhealthy", "service": service, "commit": commit})
elif returnValue and "unhealthy" not in returnValue:
create_sc_increment(name='deployd.service_health_status',
create_sc_increment(name='deployd.service_health_status',
tags={"status": "healthy", "service": service, "commit": commit})
if returnValue:
return returnValue
# if no check happens for the current service, check if it is a non container with healthcheck enabled
return redeploy_check_without_container_status(commit, service, redeploy)
except:
except Exception:
log.error(f"Failed to get container health info with commit {commit}")
return None


def get_telefig_version() -> Optional[str]:
if not IS_PINTEREST:
return None
return None
try:
cmd = ['configure-serviceset', '-v']
output = subprocess.run(cmd, check=True, stdout=subprocess.PIPE).stdout
if output:
return output.decode().strip()
else:
return None
except:
except Exception:
log.error("Error when fetching teletraan configure manager version")
return None

Expand Down
4 changes: 2 additions & 2 deletions deploy-agent/deployd/download/downloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,10 @@ def download(self) -> int:
with open(extracted_file, 'w'):
pass
log.info("Successfully extracted {} to {}".format(local_full_fn, working_dir))
except tarfile.TarError as e:
except tarfile.TarError:
status = Status.FAILED
log.exception("Failed to extract tar files")
except OSError as e:
except OSError:
status = Status.FAILED
log.exception("Failed to extract files. OSError:")
except Exception:
Expand Down
6 changes: 3 additions & 3 deletions deploy-agent/deployd/staging/transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down Expand Up @@ -70,7 +70,7 @@ def _translate(self, from_path, to_path) -> None:
res = s.safe_substitute(self._dictionary)
with open(to_path, 'w') as f:
f.write(res)
except:
except Exception:
log.error('Fail to translate script {}, stacktrace: {}'.format(from_path,
traceback.format_exc()))

Expand Down
6 changes: 3 additions & 3 deletions deploy-agent/tests/unit/deploy/client/test_base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -28,7 +28,7 @@ class MyClient(BaseClient):
pass

with self.assertRaises(TypeError):
myclient = MyClient()
MyClient()

def test_abc_equivalent_to_old(self):
"""
Expand Down
3 changes: 1 addition & 2 deletions deploy-agent/tests/unit/deploy/client/test_client.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import mock
import unittest
from tests import TestCase

from deployd.client.client import Client
from deployd.client.client import Client
from deployd.client.base_client import BaseClient
from deployd.common.config import Config

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,5 @@ def test_validate_url_without_allow_list(self, mock_get_s3_download_allow_list):
self.assertTrue(result)
mock_get_s3_download_allow_list.assert_called_once()

@mock.patch('deployd.common.config.Config.get_s3_download_allow_list')
def test_validate_url_without_allow_list(self, mock_get_s3_download_allow_list):
mock_get_s3_download_allow_list.return_value = []
result = self.downloader.validate_source()

self.assertTrue(result)
mock_get_s3_download_allow_list.assert_called_once()

if __name__ == '__main__':
unittest.main()
5 changes: 2 additions & 3 deletions deploy-agent/tests/unit/deploy/staging/test_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down Expand Up @@ -100,7 +100,6 @@ def test_translate2(self):
transformer.transform_scripts(self.template_dir, self.template_dir, self.script_dir)

fn1 = os.path.join(self.script_dir, 'test1')
fn2 = os.path.join(self.script_dir, 'test2')

value1 = "foo-foo-foo"
values = {
Expand Down
7 changes: 3 additions & 4 deletions deploy-agent/tests/unit/deploy/utils/test_exec.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,16 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import os
import time
import signal
import tempfile
import unittest
Expand Down Expand Up @@ -142,7 +141,7 @@ def test_run_command_with_shutdown_timeout(self):
executor.MAX_SLEEP_INTERVAL = 5
executor.MAX_TAIL_BYTES = 10240
executor.TERMINATE_TIMEOUT = 0
deploy_report = executor.run_cmd(cmd=cmd)
executor.run_cmd(cmd=cmd)
self.assertEqual(os.killpg.call_count, 2)
calls = [mock.call(mock.ANY, signal.SIGTERM), mock.call(mock.ANY, signal.SIGKILL)]
os.killpg.assert_has_calls(calls)
Expand Down
5 changes: 5 additions & 0 deletions deploy-agent/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ recreate=True
deps=-rrequirements_test.txt
commands=py.test --cov=deployd {posargs}

[testenv:ruff]
description = run ruff
deps = ruff
commands = ruff {posargs}

[gh-actions]
python =
3.6: py36
Expand Down
Loading