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/01/23 07:42:37 UTC
[GitHub] [airflow] VBhojawala opened a new pull request #13859: Added copy connection action
VBhojawala opened a new pull request #13859:
URL: https://github.com/apache/airflow/pull/13859
Added copy connection action which attaches __suffix to duplicated connection.
Closes : #12401
----------------------------------------------------------------
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 #13859: Added copy connection action
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13859:
URL: https://github.com/apache/airflow/pull/13859#issuecomment-812945481
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days 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] VBhojawala commented on a change in pull request #13859: Added copy connection action
Posted by GitBox <gi...@apache.org>.
VBhojawala commented on a change in pull request #13859:
URL: https://github.com/apache/airflow/pull/13859#discussion_r575768396
##########
File path: tests/www/test_views.py
##########
@@ -254,6 +254,66 @@ def test_prefill_form_null_extra(self):
cmv.prefill_form(form=mock_form, pk=1)
+class TestCopyConnectionModelView(TestBase):
+ def setUp(self):
+ super().setUp()
+ conn1 = Connection(
+ conn_id='aws_mongo',
+ conn_type='FTP',
+ description='MongoDB',
+ host='192.168.55.11',
+ schema='airflow',
+ port='5566',
+ )
+
+ # Already copied and modified
+ conn2 = Connection(
+ conn_id='aws_mongo__1',
+ conn_type='FTP',
+ description='MongoDB2',
+ host='192.168.55.12',
+ schema='airflow',
+ port='5567',
+ )
+
+ conn3 = Connection(
+ conn_id='aws_mysql',
+ conn_type='FTP',
+ description='MongoDB2',
+ host='192.168.55.12',
+ schema='airflow',
+ port='5567',
+ )
+
+ self.clear_table(Connection)
+ self.session.add_all([conn1, conn2, conn3])
+ self.session.commit()
+
+ def tearDown(self):
+ self.clear_table(Connection)
+ super().tearDown()
+
+ def test_copy_connection(self):
+ """ Test copy multiple connection with suffix(already copied )"""
+
+ mock_form = mock.Mock()
+ mock_form.data = {"action": "mulcopy", "rowid": [1, 2, 3]}
+ resp = self.client.post('/connection/action_post', data=mock_form.data, follow_redirects=True)
+
+ expected_result = {
+ 'aws_mongo',
+ 'aws_mongo__1',
+ 'aws_mongo__2',
+ 'aws_mongo__3',
Review comment:
Here in test case i have used '__1' as existing connection to test copy connection which were already copied connection and not renamed after copy.
For Pattern matching in RegEx we have used '__/d+$' to differentiate system(airflow) assigned suffix (we have used same Regex for Task and TaskGroup decorators). User can use single underscore '_' or '-' to assign number to connection name which will make it start from 1.
For Example
'mongo_1' -> 'mongo_1__1'
'mongo-1' -> 'mongo-1__1'
----------------------------------------------------------------
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] closed pull request #13859: Added copy connection action
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #13859:
URL: https://github.com/apache/airflow/pull/13859
--
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] VBhojawala commented on pull request #13859: Added copy connection action
Posted by GitBox <gi...@apache.org>.
VBhojawala commented on pull request #13859:
URL: https://github.com/apache/airflow/pull/13859#issuecomment-778741846
Re-based 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] VBhojawala commented on pull request #13859: Added copy connection action
Posted by GitBox <gi...@apache.org>.
VBhojawala commented on pull request #13859:
URL: https://github.com/apache/airflow/pull/13859#issuecomment-766293611
Added test case to check copy connection with and without prefix with multi select.
----------------------------------------------------------------
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] jhtimmins commented on a change in pull request #13859: Added copy connection action
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #13859:
URL: https://github.com/apache/airflow/pull/13859#discussion_r575766870
##########
File path: tests/www/test_views.py
##########
@@ -254,6 +254,66 @@ def test_prefill_form_null_extra(self):
cmv.prefill_form(form=mock_form, pk=1)
+class TestCopyConnectionModelView(TestBase):
+ def setUp(self):
+ super().setUp()
+ conn1 = Connection(
+ conn_id='aws_mongo',
+ conn_type='FTP',
+ description='MongoDB',
+ host='192.168.55.11',
+ schema='airflow',
+ port='5566',
+ )
+
+ # Already copied and modified
+ conn2 = Connection(
+ conn_id='aws_mongo__1',
+ conn_type='FTP',
+ description='MongoDB2',
+ host='192.168.55.12',
+ schema='airflow',
+ port='5567',
+ )
+
+ conn3 = Connection(
+ conn_id='aws_mysql',
+ conn_type='FTP',
+ description='MongoDB2',
+ host='192.168.55.12',
+ schema='airflow',
+ port='5567',
+ )
+
+ self.clear_table(Connection)
+ self.session.add_all([conn1, conn2, conn3])
+ self.session.commit()
+
+ def tearDown(self):
+ self.clear_table(Connection)
+ super().tearDown()
+
+ def test_copy_connection(self):
+ """ Test copy multiple connection with suffix(already copied )"""
+
+ mock_form = mock.Mock()
+ mock_form.data = {"action": "mulcopy", "rowid": [1, 2, 3]}
+ resp = self.client.post('/connection/action_post', data=mock_form.data, follow_redirects=True)
+
+ expected_result = {
+ 'aws_mongo',
+ 'aws_mongo__1',
+ 'aws_mongo__2',
+ 'aws_mongo__3',
Review comment:
Is it fair to treat all `aws_mongo` variants as essentially interchangeable in terms of numbering them? Would `aws_mongo__1__1` make more sense than `aws_mongo__3` to make clear that it's a copy of `aws_mongo__1`? It doesn't look as nice, but it seems like people would copy `aws_mongo__1` because they want a copy of that exact variant.
##########
File path: airflow/www/views.py
##########
@@ -2896,6 +2897,49 @@ def action_muldelete(self, items):
self.update_redirect()
return redirect(self.get_redirect())
+ @action('mulcopy', 'Copy', 'Are you sure you want to Duplicate selected connections?', single=False)
Review comment:
```suggestion
@action('mulcopy', 'Copy', 'Are you sure you want to duplicate the selected connections?', single=False)
```
##########
File path: airflow/www/views.py
##########
@@ -2896,6 +2897,49 @@ def action_muldelete(self, items):
self.update_redirect()
return redirect(self.get_redirect())
+ @action('mulcopy', 'Copy', 'Are you sure you want to Duplicate selected connections?', single=False)
+ @provide_session
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_CREATE, permissions.RESOURCE_CONNECTION),
+ ]
+ )
+ def action_mulcopy(self, connections, session=None):
+ """Copy multiple connections"""
+ for selected_con in connections:
Review comment:
It might make sense to change `selected_con` to `selected_conn` since 'connection' is abbreviated as 'conn' in the rest of your code.
##########
File path: tests/www/test_views.py
##########
@@ -254,6 +254,66 @@ def test_prefill_form_null_extra(self):
cmv.prefill_form(form=mock_form, pk=1)
+class TestCopyConnectionModelView(TestBase):
Review comment:
You may want to add `Multi` to this class name to clarify what this is testing
----------------------------------------------------------------
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 #13859: Added copy connection action
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #13859:
URL: https://github.com/apache/airflow/pull/13859#issuecomment-766284821
Can you add tests to avoid regression?
----------------------------------------------------------------
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