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 2020/06/14 18:53:21 UTC

[GitHub] [airflow] jkbngl opened a new pull request #9295: added mssql to oracle transfer operator

jkbngl opened a new pull request #9295:
URL: https://github.com/apache/airflow/pull/9295


   ---
   Make sure to mark the boxes below before creating PR: [x]
   
   Added Operator to transfer data from MSSql to Oracle, building on top of oracleToOracleTransfer operator.
   
   - [X] Description above provides context of the change
   - [X] Unit tests coverage for changes (not needed for documentation changes) -> added operator so probably also not mandatory
   - [X] Target Github ISSUE in description if exists -> does not exist
   - [X] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [X] Relevant documentation is updated including usage instructions. -> should be auto generated, else let me know and ill add documetation
   - [X] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   ---
   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).
   Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
   


----------------------------------------------------------------
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] ephraimbuddy commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-667353044


   > @ephraimbuddy rebase master in my branch?
   
   See https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#how-to-rebase-pr


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-644391378


   Hi @ephraimbuddy , maybe you can help me, what should I add for default value to my source_sql_params, I cant neither passed None, nor an empty list [] in both cases the checks fail. I dont think simply removing the default value would be the appropriate solution?


----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r439869459



##########
File path: airflow/contrib/operators/mssql_to_oracle_transfer.py
##########
@@ -0,0 +1,92 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+
+from airflow.models import BaseOperator
+from airflow.providers.microsoft.mssql.hooks.mssql import MsSqlHook
+from airflow.providers.oracle.hooks.oracle import OracleHook
+from airflow.utils.decorators import apply_defaults
+
+
+class MSSQLToOracleTransferOperator(BaseOperator):
+    """
+    Moves data from Oracle to MSSQL.
+
+
+    :param oracle_destination_conn_id: destination Oracle connection.
+    :type oracle_destination_conn_id: str
+    :param destination_table: destination table to insert rows.
+    :type destination_table: str
+    :param mssql_source_conn_id: source MSSQL connection.
+    :type mssql_source_conn_id: str
+    :param source_sql: SQL query to execute against the source MSSQL
+        database. (templated)
+    :type source_sql: str
+    :param source_sql_params: Parameters to use in sql query. (templated)
+    :type source_sql_params: dict
+    :param rows_chunk: number of rows per chunk to commit.
+    :type rows_chunk: int
+    """
+
+    template_fields = ('source_sql', 'source_sql_params')
+    ui_color = '#e08c8c'
+
+    @apply_defaults
+    def __init__(
+            self,
+            oracle_destination_conn_id,
+            destination_table,
+            mssql_source_conn_id,
+            source_sql,
+            source_sql_params=None,
+            rows_chunk=5000,
+            *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if source_sql_params is None:
+            source_sql_params = {}
+        self.oracle_destination_conn_id = oracle_destination_conn_id
+        self.destination_table = destination_table
+        self.mssql_source_conn_id = mssql_source_conn_id
+        self.source_sql = source_sql
+        self.source_sql_params = source_sql_params
+        self.rows_chunk = rows_chunk
+
+    # pylint: disable=unused-argument

Review comment:
       ```suggestion
   ```




----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-646298216


   @ephraimbuddy would you mind helping me again? The checks are failing because of:
   - Build cross-dependencies for providers packages - Failed
   ```
   -microsoft.mssql            odbc
   +microsoft.mssql            odbc,oracle
   ```
   
   I have added the dependencies to `airflow/providers/dependencies.json`, do I need to adjust them anywhere 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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-643826150


   @ephraimbuddy  thank you very much for your feedback, I have now added type annotations!


----------------------------------------------------------------
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] ephraimbuddy commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-645605008


   Hi @jkbngl.
   Sorry, I didn't notice the location of your operator before now.
   We no longer have operator in `contrib` package. Please check the `providers` package and find the appropriate provider to place your codes. 


----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r440452964



##########
File path: airflow/contrib/operators/mssql_to_oracle_transfer.py
##########
@@ -0,0 +1,92 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+

Review comment:
       from typing import Optional




----------------------------------------------------------------
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] RosterIn commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
RosterIn commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-694841342


   @jkbngl I'm interested in understanding what were the limitations with [GenericTransfer](https://github.com/apache/airflow/blob/master/airflow/operators/generic_transfer.py#L25) that resulted in writing this operator. If both approaches are valid and working then I think it's important to document when to use which.
   WDYT?


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-667352145


   @ephraimbuddy rebase master in my 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] codecov-commenter commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-669476703


   # [Codecov](https://codecov.io/gh/apache/airflow/pull/9295?src=pr&el=h1) Report
   > Merging [#9295](https://codecov.io/gh/apache/airflow/pull/9295?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/4054663769e3a3861773469469dd3efa7b383df8&el=desc) will **increase** coverage by `27.98%`.
   > The diff coverage is `43.45%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/9295/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/9295?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #9295       +/-   ##
   ===========================================
   + Coverage    6.22%   34.21%   +27.98%     
   ===========================================
     Files         946     1038       +92     
     Lines       45695    49673     +3978     
   ===========================================
   + Hits         2846    16994    +14148     
   + Misses      42849    32679    -10170     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #sqlite-tests-Integration-3.6 | `34.21% <43.45%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/9295?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/api/auth/backend/default.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2RlZmF1bHQucHk=) | `0.00% <0.00%> (-69.24%)` | :arrow_down: |
   | [airflow/api/auth/backend/deny\_all.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2RlbnlfYWxsLnB5) | `0.00% <0.00%> (ø)` | |
   | [airflow/api/client/json\_client.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY2xpZW50L2pzb25fY2xpZW50LnB5) | `0.00% <0.00%> (ø)` | |
   | [airflow/cli/commands/db\_command.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvZGJfY29tbWFuZC5weQ==) | `31.81% <ø> (+31.81%)` | :arrow_up: |
   | [airflow/cli/commands/info\_command.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvaW5mb19jb21tYW5kLnB5) | `34.97% <ø> (+34.97%)` | :arrow_up: |
   | [airflow/cli/commands/task\_command.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvdGFza19jb21tYW5kLnB5) | `21.69% <0.00%> (+21.69%)` | :arrow_up: |
   | [airflow/config\_templates/default\_celery.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb25maWdfdGVtcGxhdGVzL2RlZmF1bHRfY2VsZXJ5LnB5) | `52.94% <ø> (+52.94%)` | :arrow_up: |
   | [...rflow/config\_templates/default\_webserver\_config.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb25maWdfdGVtcGxhdGVzL2RlZmF1bHRfd2Vic2VydmVyX2NvbmZpZy5weQ==) | `0.00% <0.00%> (ø)` | |
   | [airflow/contrib/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL19faW5pdF9fLnB5) | `0.00% <0.00%> (ø)` | |
   | [...low/contrib/hooks/azure\_container\_instance\_hook.py](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2F6dXJlX2NvbnRhaW5lcl9pbnN0YW5jZV9ob29rLnB5) | `0.00% <ø> (ø)` | |
   | ... and [949 more](https://codecov.io/gh/apache/airflow/pull/9295/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/9295?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/9295?src=pr&el=footer). Last update [2b8dea6...88d6a2d](https://codecov.io/gh/apache/airflow/pull/9295?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-668197876


   Hi @ephraimbuddy I did rebase, any hint on how to continue? 


----------------------------------------------------------------
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] ephraimbuddy commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-646307048


   Also @jkbngl  you can read about it here https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#backport-providers-packages


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-654397085


   Hi @mik-laj this actually helped a lot clarified a lot of things for me! Thank you very much!
   So what I understood is that transfer operators should go into the `airflow.providers.*.transfer` folder, right? 
   I have done this, but now I get an error when building the docs:
   `/opt/airflow/docs/_api/airflow/providers/microsoft/mssql/transfer/index.rst: WARNING: document isn't included in any toctree`
   
   Might this be related?


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-670141704


   Hi @ephraimbuddy it seems like you suggestion worked, thanks a lot 👍 


----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r439869177



##########
File path: airflow/contrib/operators/mssql_to_oracle_transfer.py
##########
@@ -0,0 +1,92 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+
+from airflow.models import BaseOperator
+from airflow.providers.microsoft.mssql.hooks.mssql import MsSqlHook
+from airflow.providers.oracle.hooks.oracle import OracleHook
+from airflow.utils.decorators import apply_defaults
+
+
+class MSSQLToOracleTransferOperator(BaseOperator):
+    """
+    Moves data from Oracle to MSSQL.
+
+
+    :param oracle_destination_conn_id: destination Oracle connection.
+    :type oracle_destination_conn_id: str
+    :param destination_table: destination table to insert rows.
+    :type destination_table: str
+    :param mssql_source_conn_id: source MSSQL connection.
+    :type mssql_source_conn_id: str
+    :param source_sql: SQL query to execute against the source MSSQL
+        database. (templated)
+    :type source_sql: str
+    :param source_sql_params: Parameters to use in sql query. (templated)
+    :type source_sql_params: dict
+    :param rows_chunk: number of rows per chunk to commit.
+    :type rows_chunk: int
+    """
+
+    template_fields = ('source_sql', 'source_sql_params')
+    ui_color = '#e08c8c'
+
+    @apply_defaults
+    def __init__(
+            self,
+            oracle_destination_conn_id,
+            destination_table,
+            mssql_source_conn_id,
+            source_sql,
+            source_sql_params=None,
+            rows_chunk=5000,
+            *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if source_sql_params is None:
+            source_sql_params = {}
+        self.oracle_destination_conn_id = oracle_destination_conn_id
+        self.destination_table = destination_table
+        self.mssql_source_conn_id = mssql_source_conn_id
+        self.source_sql = source_sql
+        self.source_sql_params = source_sql_params
+        self.rows_chunk = rows_chunk
+
+    # pylint: disable=unused-argument
+    def _execute(self, src_hook, dest_hook, context):

Review comment:
       ```suggestion
       def _execute(self, src_hook, dest_hook):
   ```




----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-662653175


   Hi @mik-laj not sure if this is causing any problems, this is also not really the target for this pull request, its solely to add the very simple (mssql to oracle transfer) operator not to change the fundamental functionality of the mssql operator, which would probably need a more experiences contributor than me tbh.
   Whats your opinion?


----------------------------------------------------------------
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] codecov-commenter edited a comment on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-669476703






----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r446403869



##########
File path: tests/providers/microsoft/mssql/operators/test_mssql_to_oracle.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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 unittest
+from unittest import mock
+
+from mock import MagicMock
+
+from airflow.providers.microsoft.mssql.operators.mssql_to_oracle import MSSQLToOracleTransferOperator

Review comment:
       ```suggestion
   from airflow.providers.microsoft.mssql.operators.mssql_to_oracle import MsSqlToOracleOperator
   
   ```




----------------------------------------------------------------
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] ephraimbuddy commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-665875397






----------------------------------------------------------------
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] mik-laj commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-658975293


   I'm not an expert, but I think you can use odbc to work around this problem.
   https://github.com/apache/airflow/blob/4bde99f1323d72f6c84c1548079d5e98fc0a2a9a/airflow/providers/microsoft/mssql/hooks/mssql.py#L19-L22


----------------------------------------------------------------
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] mik-laj edited a comment on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-658975293


   I'm not an expert, but I think you can use odbc to work around this problem. Is this causing any problems?
   https://github.com/apache/airflow/blob/4bde99f1323d72f6c84c1548079d5e98fc0a2a9a/airflow/providers/microsoft/mssql/hooks/mssql.py#L19-L22


----------------------------------------------------------------
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] ephraimbuddy commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-668202527


   > Hi @ephraimbuddy I did rebase, any hint on how to continue?
   
   Consider using odbc provider as advised on the mssql hook. The pymssql has been deprecated. I didn't take a look before but here is the information on the module.
   https://github.com/apache/airflow/blob/86d8e349b4e810c0dca56d5202f893628969970a/airflow/providers/microsoft/mssql/hooks/mssql.py#L33-L36


----------------------------------------------------------------
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] stale[bot] commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-727619488


   This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
   


----------------------------------------------------------------
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] mik-laj commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-653234573


   Here is naming convention:
   https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#naming-conventions-for-provider-packages
   is this helpful for you?


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-650392351


   Hi @ephraimbuddy maybe you can help me here again, I have a strange error, that the module is not found `no-name-in-module`:
   - No name 'MSSQLToOracleTransferOperator' in module 'airflow.providers.microsoft.mssql.operators.mssql_to_oracle' (no-name-in-module)
   I thought it might be related to a missing `init `file, I added it but still get the same error... Any other idea?


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-653231283


   Hi @ephraimbuddy would you mind helping me with the naming convention of the operators? I got an error for my operator to not match: .*Operator$ so I renamed it to MsSqlToOracleOperator, now I get an error for matching .*To[A-Z0-9].*Operator$, the issue is the "**To**" from my understanding... 
   What would be the correct name for my operator? I also checked other operators and they also match .*To[A-Z0-9].*Operator$, like e.g.:
   - FileToWasbOperator
   - OracleToAzureDataLakeTransferOperator
   - OracleToOracleTransferOperator
   - ....
   
   Maybe it would be correct to move it in a transfer folder instead of in the operator folder, but this is also not the case for all other transfer operators? Your help would be appreciated! Thanks


----------------------------------------------------------------
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] mik-laj commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-665861273


   potiuk is on vacation.


----------------------------------------------------------------
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] mik-laj commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-653235179


   Here is voting and discussion about transfer package: https://lists.apache.org/x/thread.html/r3514ef575b437b9eb368111b1e4b03ad7455e63d64c359c22fd6ea9a@%3Cdev.airflow.apache.org%3E


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-665859818


   @ephraimbuddy or @potiuk what do you think about my last commet?


----------------------------------------------------------------
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] jkbngl edited a comment on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl edited a comment on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-665859818


   @ephraimbuddy or @potiuk what do you think about my last comment?


----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r439869177



##########
File path: airflow/contrib/operators/mssql_to_oracle_transfer.py
##########
@@ -0,0 +1,92 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+
+from airflow.models import BaseOperator
+from airflow.providers.microsoft.mssql.hooks.mssql import MsSqlHook
+from airflow.providers.oracle.hooks.oracle import OracleHook
+from airflow.utils.decorators import apply_defaults
+
+
+class MSSQLToOracleTransferOperator(BaseOperator):
+    """
+    Moves data from Oracle to MSSQL.
+
+
+    :param oracle_destination_conn_id: destination Oracle connection.
+    :type oracle_destination_conn_id: str
+    :param destination_table: destination table to insert rows.
+    :type destination_table: str
+    :param mssql_source_conn_id: source MSSQL connection.
+    :type mssql_source_conn_id: str
+    :param source_sql: SQL query to execute against the source MSSQL
+        database. (templated)
+    :type source_sql: str
+    :param source_sql_params: Parameters to use in sql query. (templated)
+    :type source_sql_params: dict
+    :param rows_chunk: number of rows per chunk to commit.
+    :type rows_chunk: int
+    """
+
+    template_fields = ('source_sql', 'source_sql_params')
+    ui_color = '#e08c8c'
+
+    @apply_defaults
+    def __init__(
+            self,
+            oracle_destination_conn_id,
+            destination_table,
+            mssql_source_conn_id,
+            source_sql,
+            source_sql_params=None,
+            rows_chunk=5000,
+            *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if source_sql_params is None:
+            source_sql_params = {}
+        self.oracle_destination_conn_id = oracle_destination_conn_id
+        self.destination_table = destination_table
+        self.mssql_source_conn_id = mssql_source_conn_id
+        self.source_sql = source_sql
+        self.source_sql_params = source_sql_params
+        self.rows_chunk = rows_chunk
+
+    # pylint: disable=unused-argument
+    def _execute(self, src_hook, dest_hook, context):

Review comment:
       ```suggestion
       def _execute(self, src_hook: str, dest_hook: str):
   ```




----------------------------------------------------------------
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] potiuk commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-647133953


   As the failing tests explain now - you need to add some tests to the operator @jkbngl :)


----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r440452590



##########
File path: airflow/contrib/operators/mssql_to_oracle_transfer.py
##########
@@ -0,0 +1,92 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+
+from airflow.models import BaseOperator
+from airflow.providers.microsoft.mssql.hooks.mssql import MsSqlHook
+from airflow.providers.oracle.hooks.oracle import OracleHook
+from airflow.utils.decorators import apply_defaults
+
+
+class MSSQLToOracleTransferOperator(BaseOperator):
+    """
+    Moves data from Oracle to MSSQL.
+
+
+    :param oracle_destination_conn_id: destination Oracle connection.
+    :type oracle_destination_conn_id: str
+    :param destination_table: destination table to insert rows.
+    :type destination_table: str
+    :param mssql_source_conn_id: source MSSQL connection.
+    :type mssql_source_conn_id: str
+    :param source_sql: SQL query to execute against the source MSSQL
+        database. (templated)
+    :type source_sql: str
+    :param source_sql_params: Parameters to use in sql query. (templated)
+    :type source_sql_params: dict
+    :param rows_chunk: number of rows per chunk to commit.
+    :type rows_chunk: int
+    """
+
+    template_fields = ('source_sql', 'source_sql_params')
+    ui_color = '#e08c8c'
+
+    @apply_defaults
+    def __init__(
+            self,
+            oracle_destination_conn_id: str,
+            destination_table: str,
+            mssql_source_conn_id: str,
+            source_sql: str,
+            source_sql_params: dict = [],

Review comment:
       ```suggestion
               source_sql_params: Optional[dict] = None,
   ```




----------------------------------------------------------------
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] ephraimbuddy commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-644420454


   You may want to look at pre-commit hooks, so you can always avoid static check errors
   
   https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#pre-commit-hooks


----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r439869418



##########
File path: airflow/contrib/operators/mssql_to_oracle_transfer.py
##########
@@ -0,0 +1,92 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+
+from airflow.models import BaseOperator
+from airflow.providers.microsoft.mssql.hooks.mssql import MsSqlHook
+from airflow.providers.oracle.hooks.oracle import OracleHook
+from airflow.utils.decorators import apply_defaults
+
+
+class MSSQLToOracleTransferOperator(BaseOperator):
+    """
+    Moves data from Oracle to MSSQL.
+
+
+    :param oracle_destination_conn_id: destination Oracle connection.
+    :type oracle_destination_conn_id: str
+    :param destination_table: destination table to insert rows.
+    :type destination_table: str
+    :param mssql_source_conn_id: source MSSQL connection.
+    :type mssql_source_conn_id: str
+    :param source_sql: SQL query to execute against the source MSSQL
+        database. (templated)
+    :type source_sql: str
+    :param source_sql_params: Parameters to use in sql query. (templated)
+    :type source_sql_params: dict
+    :param rows_chunk: number of rows per chunk to commit.
+    :type rows_chunk: int
+    """
+
+    template_fields = ('source_sql', 'source_sql_params')
+    ui_color = '#e08c8c'
+
+    @apply_defaults
+    def __init__(
+            self,
+            oracle_destination_conn_id,
+            destination_table,
+            mssql_source_conn_id,
+            source_sql,
+            source_sql_params=None,
+            rows_chunk=5000,

Review comment:
       Would be nice if you could add type annotations




----------------------------------------------------------------
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] potiuk commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-646302338


   If you look at the error message you will see what's going on @jkbngl 
   
    --- a/CONTRIBUTING.rst
   +++ b/CONTRIBUTING.rst
   @@ -462,7 +462,7 @@ discord                    http
    google                     amazon,apache.cassandra,cncf.kubernetes,facebook,microsoft.azure,microsoft.mssql,mysql,postgres,presto,sftp
    hashicorp                  google
    microsoft.azure            oracle
   -microsoft.mssql            odbc
   +microsoft.mssql            odbc,oracle
    mysql                      amazon,presto,vertica
    opsgenie                   http
    postgres                   amazon
   
   
   It's the CBTRIBUTING.rst that gets updated. 
   
   But you do not have to do much - just install pre-commts and wnen you commit all the changes will be automatically made for you - you will just need to commit what pre-commit will modify.


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-658972831


   Hi @mik-laj it seems like 9 failing checks are all due to:
   `
   ModuleNotFoundError: No module named 'pymssql'
   `
   In the `airflow/providers/microsoft/mssql/hooks/mssql.py`, I suggest this is because the pymssql is going to become deprecated? Is there something I can do about this error?


----------------------------------------------------------------
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] ephraimbuddy commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-645617767


   Check other operators in provider package and see how they are named, just remove the transfer appended to it. I suggest to put the operator in `providers/microsoft/mssql/operators`. Also don't forget to add it to documentation  at 'docs/operators-and-hooks-ref' and `docs/howto`. Let me know if you need any help


----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r446403869



##########
File path: tests/providers/microsoft/mssql/operators/test_mssql_to_oracle.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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 unittest
+from unittest import mock
+
+from mock import MagicMock
+
+from airflow.providers.microsoft.mssql.operators.mssql_to_oracle import MSSQLToOracleTransferOperator

Review comment:
       ```suggestion
   from airflow.providers.microsoft.mssql.operators.mssql_to_oracle import MsSqlToOracleOperator
   
   ```
   You no longer have `MSSQLToOracleTransferOperator`, I guess you renamed it




----------------------------------------------------------------
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] ephraimbuddy removed a comment on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy removed a comment on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-665875397


   Hi @jkbngl , have you tried to use odbc?


----------------------------------------------------------------
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] jkbngl commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
jkbngl commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-645586989


   @ephraimbuddy could you help me again please, I am getting following errors in the checks, which I dont really understand on how to act on:
   - FAILED tests/test_project_structure.py::TestProjectStructure::test_deprecated_packages
   - FAILED tests/models/test_dagcode.py::TestDagCode::test_remove_unused_code - A...
   - ERROR: Integration rabbitmq could not be started!
   
   I dont really see the connection between my added connector, which is in the same folder as all other contrib operators and a failing projectstructure? Should I maybe move the operator in the providers folder?
   
   Same for the test_dagcode:test_remove_unused_code -> Deletes code that no longer has any DAGs referencing it - not sure what I should do to fix this error?
   
   And for the last, I dont know how rabbitmq is affected by me adding an operator? 
   
   @ephraimbuddy It would be great if you could give me some hints in how to fix this errors!
   


----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r440452964



##########
File path: airflow/contrib/operators/mssql_to_oracle_transfer.py
##########
@@ -0,0 +1,92 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+

Review comment:
       ```suggestion
   from typing import Optional
   ```




----------------------------------------------------------------
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] ephraimbuddy commented on a change in pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#discussion_r439869251



##########
File path: airflow/contrib/operators/mssql_to_oracle_transfer.py
##########
@@ -0,0 +1,92 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+
+from airflow.models import BaseOperator
+from airflow.providers.microsoft.mssql.hooks.mssql import MsSqlHook
+from airflow.providers.oracle.hooks.oracle import OracleHook
+from airflow.utils.decorators import apply_defaults
+
+
+class MSSQLToOracleTransferOperator(BaseOperator):
+    """
+    Moves data from Oracle to MSSQL.
+
+
+    :param oracle_destination_conn_id: destination Oracle connection.
+    :type oracle_destination_conn_id: str
+    :param destination_table: destination table to insert rows.
+    :type destination_table: str
+    :param mssql_source_conn_id: source MSSQL connection.
+    :type mssql_source_conn_id: str
+    :param source_sql: SQL query to execute against the source MSSQL
+        database. (templated)
+    :type source_sql: str
+    :param source_sql_params: Parameters to use in sql query. (templated)
+    :type source_sql_params: dict
+    :param rows_chunk: number of rows per chunk to commit.
+    :type rows_chunk: int
+    """
+
+    template_fields = ('source_sql', 'source_sql_params')
+    ui_color = '#e08c8c'
+
+    @apply_defaults
+    def __init__(
+            self,
+            oracle_destination_conn_id,
+            destination_table,
+            mssql_source_conn_id,
+            source_sql,
+            source_sql_params=None,
+            rows_chunk=5000,
+            *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if source_sql_params is None:
+            source_sql_params = {}
+        self.oracle_destination_conn_id = oracle_destination_conn_id
+        self.destination_table = destination_table
+        self.mssql_source_conn_id = mssql_source_conn_id
+        self.source_sql = source_sql
+        self.source_sql_params = source_sql_params
+        self.rows_chunk = rows_chunk
+
+    # pylint: disable=unused-argument
+    def _execute(self, src_hook, dest_hook, context):
+        with src_hook.get_conn() as src_conn:
+            cursor = src_conn.cursor()
+            self.log.info("Querying data from source: %s", self.mssql_source_conn_id)
+            cursor.execute(self.source_sql, self.source_sql_params)
+            target_fields = list(map(lambda field: field[0], cursor.description))
+
+            rows_total = 0
+            rows = cursor.fetchmany(self.rows_chunk)
+            while len(rows) > 0:
+                rows_total = rows_total + len(rows)
+                dest_hook.bulk_insert_rows(self.destination_table, rows,
+                                           target_fields=target_fields,
+                                           commit_every=self.rows_chunk)
+                rows = cursor.fetchmany(self.rows_chunk)
+                self.log.info("Total inserted: %s rows", rows_total)
+
+            self.log.info("Finished data transfer.")
+            cursor.close()
+
+    def execute(self, context):
+        src_hook = MsSqlHook(mssql_conn_id=self.mssql_source_conn_id)
+        dest_hook = OracleHook(oracle_conn_id=self.oracle_destination_conn_id)
+        self._execute(src_hook, dest_hook, context)

Review comment:
       ```suggestion
           self._execute(src_hook, dest_hook)
   ```




----------------------------------------------------------------
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] stale[bot] closed pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #9295:
URL: https://github.com/apache/airflow/pull/9295


   


----------------------------------------------------------------
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] mik-laj commented on pull request #9295: added mssql to oracle transfer operator

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9295:
URL: https://github.com/apache/airflow/pull/9295#issuecomment-654400905


   You need to add this package to index:
   https://github.com/apache/airflow/blob/master/docs/autoapi_templates/index.rst


----------------------------------------------------------------
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