-
Notifications
You must be signed in to change notification settings - Fork 241
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
Delete even more mesos code #3876
base: master
Are you sure you want to change the base?
Conversation
…ted from that module, to the place where it was imported.
…ainer function to local_run which still uses it.
…d code within firewall.py
…a_tools/mesos and associated tests
af58b23
to
cbe056b
Compare
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.
is this changing cause further in this pr we're switching to using just deploymentsv2v
if so,are we ready to use deploymentsv2? i vaguely remember there being some issues last time we tried to switch to using v2 for everything
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.
Vulture seems to think nothing is using deployments v1 (at least the mypy types). I'm not sure I believe that nothing is using the v1 part of the json, but I'm leaving the file format alone anyway.
@@ -79,6 +79,28 @@ class AWSSessionCreds(TypedDict): | |||
AWS_SECURITY_TOKEN: str | |||
|
|||
|
|||
def execute_in_container(docker_client, container_id, cmd, timeout): |
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 assume this is a move, but is it me or is timeout
unused?
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.
if we clean up the unused param, we can probably also
type this
def execute_in_container(docker_client, container_id, cmd, timeout): | |
def execute_in_container(docker_client: docker.Client, container_id: str, cmd: str) -> Tuple[str, int]: |
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.
just double-checking: there's no files in soaconfigs referencing these dead options?
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.
!8ball pull https://github.com/Yelp/paasta/pull/3772/files into this branch
@@ -40,81 +36,9 @@ class TaskAllocationInfo(NamedTuple): | |||
host_ip: str | |||
git_sha: str | |||
config_sha: str | |||
mesos_container_id: str # Because Mesos task info does not have docker id |
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.
might want to double-check if any of the splunk queries that we have will break if this field isn't present
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.
(it's probably safe since it's been null for ages, but you never know :p)
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.
with the tickets jfong found, i'm now like 99% certain we can cleanup soaconfigs and then delete this code entirely since it's not actually doing anything
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.
once we release the de-meso'd tron, we can probably also delete all the remaining mesos code in setup_tron_namespace :p
This one isn't pure deletion, I moved a few things around.