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/06/01 16:19:16 UTC
[GitHub] [airflow] malthe opened a new pull request #16115: Add support for extra parameters to samba client
malthe opened a new pull request #16115:
URL: https://github.com/apache/airflow/pull/16115
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
This adds the option to use the _extra_ field for adding additional parameters to the `SambaClient` constructor such as `workgroup` using JSON-formatting.
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe commented on pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#issuecomment-854443968
@eladkal fixed – I don't really understand this automated build, but the static test seems to be skipped now.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r640751662
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
There is an open issue: https://github.com/apache/airflow/issues/14054
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal merged pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
eladkal merged pull request #16115:
URL: https://github.com/apache/airflow/pull/16115
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe closed pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe closed pull request #16115:
URL: https://github.com/apache/airflow/pull/16115
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#issuecomment-854399039
@malthe can you fix the static test?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r644525992
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@malthe
Can you please rebase to latest master?
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@malthe
Can you please rebase to latest main (former master branch)?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#issuecomment-854399039
@malthe can you fix the static test?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#issuecomment-853784709
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r644503143
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@uranusjr done
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r644527978
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@eladkal done!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r644525992
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@malthe
Can you please rebase to latest master?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r645752068
##########
File path: tests/providers/airbyte/hooks/test_airbyte.py
##########
@@ -128,7 +128,7 @@ def test_wait_for_job_cancelled(self, mock_get_job):
@requests_mock.mock()
def test_connection_success(self, m):
- m.get(self.health_endpoint, status_code=200,)
+ m.get(self.health_endpoint, status_code=200)
Review comment:
This change is not related :)
We also had a broken CI so another rebase so fix the failing tests
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r643253127
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
Yup LGTM. Could you close-reopen the PR to trigger CI?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r640661226
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
Is the list of fields exhaustive? I feel we should do some sanitising instead of passing the whole JSON object directly in.
Also `:param` is for function arguments and should not be used for `extra` keys. These descriptions should live somewhere else.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r643243948
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@uranusjr so I think we can perhaps go ahead and consider merging this request and then I'll submit another request to migrate to `pysmb`.
I have already done some testing and it seems very well suited.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r640689998
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@uranusjr fixed in 0c8cca7 – the list is inexhaustive (mentioned now) and I have reformatted as definition list.
The `:param` stuff was token from the Oracle hook, but I guess that is a bad example :-)
By the way, I have since realized that the underlying Python library that supports `SambaHook` is basically abandonware and not particularly useful – it breaks quite a few expectations if you look at the actual `smbclient` functionality.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r644615325
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@eladkal looks about ready now, checks have run.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe commented on pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#issuecomment-854443968
@eladkal fixed – I don't really understand this automated build, but the static test seems to be skipped now.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r644503143
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@uranusjr done
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@eladkal done!
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@eladkal looks about ready now, checks have run.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#issuecomment-853784709
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r644525992
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
@malthe
Can you please rebase to latest main (former master branch)?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#discussion_r640694138
##########
File path: airflow/providers/samba/hooks/samba.py
##########
@@ -36,12 +36,26 @@ def __init__(self, samba_conn_id: str = default_conn_name) -> None:
self.conn = self.get_connection(samba_conn_id)
def get_conn(self) -> SambaClient:
+ """
+ Return a samba client object.
+
+ You can provide optional parameters in the extra fields of
+ your connection.
+
+ :param logdir: Base directory name for log/debug files.
+ :param kerberos: Try to authenticate with kerberos.
+ :param workgroup: Set the SMB domain of the username.
+ :param netbios_name:
+ This option allows you to override the NetBIOS name that
+ Samba uses for itself.
+ """
samba = SambaClient(
server=self.conn.host,
share=self.conn.schema,
username=self.conn.login,
ip=self.conn.host,
password=self.conn.password,
+ **self.conn.extra_dejson,
Review comment:
> By the way, I have since realized that the underlying Python library that supports `SambaHook` is basically abandonware and not particularly useful
Thanks for taising this. It would be worthwhile to open an issue for this to migrate to another library, I think. I don’t have much experience dealing with SMB with Python, but `pysmb` seems to be a popular choice.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] malthe commented on pull request #16115: Add support for extra parameters to samba client
Posted by GitBox <gi...@apache.org>.
malthe commented on pull request #16115:
URL: https://github.com/apache/airflow/pull/16115#issuecomment-855184604
@eladkal should be ready now.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org