You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/02 16:21:51 UTC
[GitHub] [airflow] nshetty15 opened a new pull request #19981: Add a retry with wait interval for SSH operator #14489
nshetty15 opened a new pull request #19981:
URL: https://github.com/apache/airflow/pull/19981
closes: #14489
Retry attempts are added if there are any errors connecting or establishing SSH session.
This will NOT retry failures like wrong host, auth errors, socket timeouts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] nshetty15 closed pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
nshetty15 closed pull request #19981:
URL: https://github.com/apache/airflow/pull/19981
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] nshetty15 closed pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
nshetty15 closed pull request #19981:
URL: https://github.com/apache/airflow/pull/19981
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19981:
URL: https://github.com/apache/airflow/pull/19981#issuecomment-1025495355
I think it was an intermittent problem. Close/Reopen triggers rebuild in this case - you can always do it yourself or commit --amend and push when you suspect the problem is "flakiness"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19981:
URL: https://github.com/apache/airflow/pull/19981#discussion_r762444304
##########
File path: airflow/providers/ssh/hooks/ssh.py
##########
@@ -257,58 +259,74 @@ def get_conn(self) -> paramiko.SSHClient:
:rtype: paramiko.client.SSHClient
"""
self.log.debug('Creating SSH client for conn_id: %s', self.ssh_conn_id)
- client = paramiko.SSHClient()
- if not self.allow_host_key_change:
- self.log.warning(
- "Remote Identification Change is not verified. "
- "This won't protect against Man-In-The-Middle attacks"
- )
- client.load_system_host_keys()
+ max_time_to_wait = 10
+ for time_to_wait in self._exponential_sleep_generator(initial=1, maximum=max_time_to_wait):
+ try:
+ client = paramiko.SSHClient()
- if self.no_host_key_check:
- self.log.warning("No Host Key Verification. This won't protect against Man-In-The-Middle attacks")
- # Default is RejectPolicy
- client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
- else:
- if self.host_key is not None:
- client_host_keys = client.get_host_keys()
- if self.port == SSH_PORT:
- client_host_keys.add(self.remote_host, self.host_key.get_name(), self.host_key)
- else:
- client_host_keys.add(
- f"[{self.remote_host}]:{self.port}", self.host_key.get_name(), self.host_key
+ if not self.allow_host_key_change:
Review comment:
I think we shoudl move it up to before the loop. Otherwise it will keep on repeating in the logs when ssh fails.
##########
File path: airflow/providers/ssh/hooks/ssh.py
##########
@@ -257,58 +259,74 @@ def get_conn(self) -> paramiko.SSHClient:
:rtype: paramiko.client.SSHClient
"""
self.log.debug('Creating SSH client for conn_id: %s', self.ssh_conn_id)
- client = paramiko.SSHClient()
- if not self.allow_host_key_change:
- self.log.warning(
- "Remote Identification Change is not verified. "
- "This won't protect against Man-In-The-Middle attacks"
- )
- client.load_system_host_keys()
+ max_time_to_wait = 10
+ for time_to_wait in self._exponential_sleep_generator(initial=1, maximum=max_time_to_wait):
+ try:
+ client = paramiko.SSHClient()
- if self.no_host_key_check:
- self.log.warning("No Host Key Verification. This won't protect against Man-In-The-Middle attacks")
- # Default is RejectPolicy
- client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
- else:
- if self.host_key is not None:
- client_host_keys = client.get_host_keys()
- if self.port == SSH_PORT:
- client_host_keys.add(self.remote_host, self.host_key.get_name(), self.host_key)
- else:
- client_host_keys.add(
- f"[{self.remote_host}]:{self.port}", self.host_key.get_name(), self.host_key
+ if not self.allow_host_key_change:
+ self.log.warning(
+ "Remote Identification Change is not verified. "
+ "This won't protect against Man-In-The-Middle attacks"
)
- else:
- pass # will fallback to system host keys if none explicitly specified in conn extra
-
- connect_kwargs = dict(
- hostname=self.remote_host,
- username=self.username,
- timeout=self.conn_timeout,
- compress=self.compress,
- port=self.port,
- sock=self.host_proxy,
- look_for_keys=self.look_for_keys,
- )
-
- if self.password:
- password = self.password.strip()
- connect_kwargs.update(password=password)
-
- if self.pkey:
- connect_kwargs.update(pkey=self.pkey)
-
- if self.key_file:
- connect_kwargs.update(key_filename=self.key_file)
-
- client.connect(**connect_kwargs)
+ client.load_system_host_keys()
- if self.keepalive_interval:
- client.get_transport().set_keepalive(self.keepalive_interval)
-
- self.client = client
- return client
+ if self.no_host_key_check:
Review comment:
Same here.
##########
File path: airflow/providers/ssh/hooks/ssh.py
##########
@@ -257,58 +259,74 @@ def get_conn(self) -> paramiko.SSHClient:
:rtype: paramiko.client.SSHClient
"""
self.log.debug('Creating SSH client for conn_id: %s', self.ssh_conn_id)
- client = paramiko.SSHClient()
- if not self.allow_host_key_change:
- self.log.warning(
- "Remote Identification Change is not verified. "
- "This won't protect against Man-In-The-Middle attacks"
- )
- client.load_system_host_keys()
+ max_time_to_wait = 10
+ for time_to_wait in self._exponential_sleep_generator(initial=1, maximum=max_time_to_wait):
Review comment:
We should use tenacity here (we already use it elsewhere).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] nshetty15 commented on a change in pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
nshetty15 commented on a change in pull request #19981:
URL: https://github.com/apache/airflow/pull/19981#discussion_r778848170
##########
File path: airflow/providers/ssh/hooks/ssh.py
##########
@@ -257,58 +259,74 @@ def get_conn(self) -> paramiko.SSHClient:
:rtype: paramiko.client.SSHClient
"""
self.log.debug('Creating SSH client for conn_id: %s', self.ssh_conn_id)
- client = paramiko.SSHClient()
- if not self.allow_host_key_change:
- self.log.warning(
- "Remote Identification Change is not verified. "
- "This won't protect against Man-In-The-Middle attacks"
- )
- client.load_system_host_keys()
+ max_time_to_wait = 10
+ for time_to_wait in self._exponential_sleep_generator(initial=1, maximum=max_time_to_wait):
Review comment:
okay.. let me change this to use tenacity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] nshetty15 commented on pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
nshetty15 commented on pull request #19981:
URL: https://github.com/apache/airflow/pull/19981#issuecomment-1026497602
@potiuk could you review this change?
I've noticed intermittent banner issues with ssh (postgres integration) when running tests on the cloud CI. Is this expected?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19981:
URL: https://github.com/apache/airflow/pull/19981#issuecomment-1026638810
Cool. I am **just** preparing to release next wave of provtders. Sometimes we have flakiness. we actively fight with it but it's an uphill (and constant) battle (however recently we have many more "green" builds than we used to have.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] nshetty15 closed pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
nshetty15 closed pull request #19981:
URL: https://github.com/apache/airflow/pull/19981
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] nshetty15 commented on pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
nshetty15 commented on pull request #19981:
URL: https://github.com/apache/airflow/pull/19981#issuecomment-986054768
@eladkal Not sure why this build failed here.. how can this be fixed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #19981:
URL: https://github.com/apache/airflow/pull/19981
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] nshetty15 commented on pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
nshetty15 commented on pull request #19981:
URL: https://github.com/apache/airflow/pull/19981#issuecomment-1025405592
@potiuk could you please help with the build failure here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #19981:
URL: https://github.com/apache/airflow/pull/19981
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19981: Add a retry with wait interval for SSH operator #14489
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19981:
URL: https://github.com/apache/airflow/pull/19981#issuecomment-986058017
That was an intermittent problem - but there are some things to addresss, so let's see next time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org