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/22 04:43:23 UTC
[GitHub] [airflow] vanka56 opened a new pull request #9472: Add drop_partition functionality for HiveMetastoreHook
vanka56 opened a new pull request #9472:
URL: https://github.com/apache/airflow/pull/9472
Adding drop partition method for HiveMetastoreHook. This becomes handy for operators involving Hive operations. Added a a unit test case as well.
Make sure to mark the boxes below before creating PR: [x]
- [ x] Description above provides context of the change
- [ x] Unit tests coverage for changes (not needed for documentation changes)
- [ x] Target Github ISSUE in description if exists
- [ 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.
- [ 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] vanka56 commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r451859081
##########
File path: tests/providers/apache/hive/hooks/test_hive.py
##########
@@ -383,6 +383,10 @@ def test_table_exists(self):
self.hook.table_exists(str(random.randint(1, 10000)))
)
+ def test_drop_partition(self):
+ self.assertTrue(self.hook.drop_partitions(self.table, db=self.database,
+ part_vals=[DEFAULT_DATE_DS]))
+
Review comment:
Sounds fair. Let me get this changed :)
----------------------------------------------------------------
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] vanka56 commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r449299122
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -775,6 +775,23 @@ def table_exists(self, table_name, db='default'):
except Exception: # pylint: disable=broad-except
return False
+ def drop_partitions(self, table_name, part_vals, delete_data=False, db='default'):
Review comment:
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] vanka56 commented on pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-652692779
@jhtimmins @turbaszek all static checks have passed now. is it good 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] vanka56 commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r456321846
##########
File path: tests/providers/apache/hive/hooks/test_hive.py
##########
@@ -557,6 +557,11 @@ def test_table_exists(self):
self.hook.metastore.__enter__().get_table.assert_called_with(
dbname='default', tbl_name='does-not-exist')
+ @mock.patch('airflow.providers.apache.hive.hooks.hive.HiveMetastoreHook.drop_partitions')
+ def test_drop_partition(self, thrift_mock):
+ self.hook.drop_partitions(self.table, db=self.database, part_vals=[DEFAULT_DATE_DS])
+ thrift_mock.assert_called_once_with(self.table, db=self.database, part_vals=[DEFAULT_DATE_DS])
+
Review comment:
@turbaszek Lol. i just laughed at my previous change(and your comment) :). Made further changed as per your advice. Let me know what do you think
----------------------------------------------------------------
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] Acehaidrey edited a comment on pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
Acehaidrey edited a comment on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-650947153
@vanka56
```airflow/providers/apache/hive/hooks/hive.py:789:30: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
airflow/providers/apache/hive/hooks/hive.py:793:26: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
```
still
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r448915531
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -775,6 +775,23 @@ def table_exists(self, table_name, db='default'):
except Exception: # pylint: disable=broad-except
return False
+ def drop_partitions(self, table_name, part_vals, delete_data=False, db='default'):
Review comment:
Please 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] turbaszek commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r448915752
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -775,6 +775,23 @@ def table_exists(self, table_name, db='default'):
except Exception: # pylint: disable=broad-except
return False
+ def drop_partitions(self, table_name, part_vals, delete_data=False, db='default'):
+ """
+ Drop partitions matching param_names input
+ >>> hh = HiveMetastoreHook()
+ >>> hh.drop_partitions(db='airflow', table_name='static_babynames',
+ part_vals="['2020-05-01']")
+ True
Review comment:
Please add arguments information using `:param xxx:` and `:type xxx:`
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r455681698
##########
File path: tests/providers/apache/hive/hooks/test_hive.py
##########
@@ -557,6 +557,11 @@ def test_table_exists(self):
self.hook.metastore.__enter__().get_table.assert_called_with(
dbname='default', tbl_name='does-not-exist')
+ @mock.patch('airflow.providers.apache.hive.hooks.hive.HiveMetastoreHook.drop_partitions')
+ def test_drop_partition(self, thrift_mock):
+ self.hook.drop_partitions(self.table, db=self.database, part_vals=[DEFAULT_DATE_DS])
+ thrift_mock.assert_called_once_with(self.table, db=self.database, part_vals=[DEFAULT_DATE_DS])
+
Review comment:
This test now check if `drop_partitions` was called. And it was a line above the assertion :)
To use mock we should mock the metastore client, because we believe it was tested and works as expected.So I would suggest something like this:
```python
@mock.patch('airflow.providers.apache.hive.hooks.hive.HiveMetastoreHook.table_exists')
@mock.patch('airflow.providers.apache.hive.hooks.hive.HiveMetastoreHook.metastore')
def test_drop_partition(self, metastore_mock, table_exist_mock):
# Here we mock behaviour of `with self.metastore as client`
client_drop_partition = metastore_mock.__enter__.return_value
# Here we want to be sure that we enter the right place of if clause
table_exist_mock.return_value = True
# Here we call the method
self.hook.drop_partitions(self.table, db=self.database, part_vals=[DEFAULT_DATE_DS])
# First lets check if we check if table exists
table_exist_mock.assert_called_once_with(self.table, self.database)
# And now we check if the underlying client.drop_partition method was called
client_drop_partition.assert_called_once_with(self.table, db=self.database, part_vals=[DEFAULT_DATE_DS])
```
The comments can be skipped, I'm also not sure if the `__enter__` mock is 100% right (something like this for sure). In case of any questions I'm happy to 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] vanka56 commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r449187394
##########
File path: tests/providers/apache/hive/hooks/test_hive.py
##########
@@ -383,6 +383,10 @@ def test_table_exists(self):
self.hook.table_exists(str(random.randint(1, 10000)))
)
+ def test_drop_partition(self):
+ self.assertTrue(self.hook.drop_partitions(self.table, db=self.database,
+ part_vals=[DEFAULT_DATE_DS]))
+
Review comment:
@turbaszek Yes. it uses Hivemetastore Thrift client. it does the partition from the test table set up for unit 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] vanka56 commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r449299190
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -775,6 +775,23 @@ def table_exists(self, table_name, db='default'):
except Exception: # pylint: disable=broad-except
return False
+ def drop_partitions(self, table_name, part_vals, delete_data=False, db='default'):
+ """
+ Drop partitions matching param_names input
+ >>> hh = HiveMetastoreHook()
+ >>> hh.drop_partitions(db='airflow', table_name='static_babynames',
+ part_vals="['2020-05-01']")
+ True
Review comment:
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] turbaszek commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r449392019
##########
File path: tests/providers/apache/hive/hooks/test_hive.py
##########
@@ -383,6 +383,10 @@ def test_table_exists(self):
self.hook.table_exists(str(random.randint(1, 10000)))
)
+ def test_drop_partition(self):
+ self.assertTrue(self.hook.drop_partitions(self.table, db=self.database,
+ part_vals=[DEFAULT_DATE_DS]))
+
Review comment:
Do you think we can mock the client? In this way we will reduce the side effects and the test will not require any external service
----------------------------------------------------------------
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] vanka56 commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r449715800
##########
File path: tests/providers/apache/hive/hooks/test_hive.py
##########
@@ -383,6 +383,10 @@ def test_table_exists(self):
self.hook.table_exists(str(random.randint(1, 10000)))
)
+ def test_drop_partition(self):
+ self.assertTrue(self.hook.drop_partitions(self.table, db=self.database,
+ part_vals=[DEFAULT_DATE_DS]))
+
Review comment:
@turbaszek Methods like def test_max_partition(self) also does the same. Moreover, it should not be an expensive operation. Do you think we really have to mock the call?
----------------------------------------------------------------
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] vanka56 commented on pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-649888490
@jhtimmins i tried that. its still failing. do you know why
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r448915957
##########
File path: tests/providers/apache/hive/hooks/test_hive.py
##########
@@ -383,6 +383,10 @@ def test_table_exists(self):
self.hook.table_exists(str(random.randint(1, 10000)))
)
+ def test_drop_partition(self):
+ self.assertTrue(self.hook.drop_partitions(self.table, db=self.database,
+ part_vals=[DEFAULT_DATE_DS]))
+
Review comment:
Does this tests requires some external service? Does it create side effects?
----------------------------------------------------------------
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] vanka56 commented on pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-650945948
@Acehaidrey Thank you! I corrected those errors 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] vanka56 commented on pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-649900700
@ashb Why are the checks always failing?
----------------------------------------------------------------
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 pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-647790321
Looks like this is failing static checks. I recommend running pre-commit checks
----------------------------------------------------------------
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] vanka56 commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r456321846
##########
File path: tests/providers/apache/hive/hooks/test_hive.py
##########
@@ -557,6 +557,11 @@ def test_table_exists(self):
self.hook.metastore.__enter__().get_table.assert_called_with(
dbname='default', tbl_name='does-not-exist')
+ @mock.patch('airflow.providers.apache.hive.hooks.hive.HiveMetastoreHook.drop_partitions')
+ def test_drop_partition(self, thrift_mock):
+ self.hook.drop_partitions(self.table, db=self.database, part_vals=[DEFAULT_DATE_DS])
+ thrift_mock.assert_called_once_with(self.table, db=self.database, part_vals=[DEFAULT_DATE_DS])
+
Review comment:
@turbaszek Lol :). Made further changed as per your advice. Let me know what do you think
----------------------------------------------------------------
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] Acehaidrey commented on pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
Acehaidrey commented on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-649713645
@vanka56 can you rebase and try the static tests again
----------------------------------------------------------------
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] vanka56 commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
vanka56 commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r443823716
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -775,6 +775,20 @@ def table_exists(self, table_name, db='default'):
except Exception: # pylint: disable=broad-except
return False
+ def drop_partitions(self, table_name, part_vals, delete_data=False, db='default'):
+ """
+ Drop partitions matching param_names input
+ >>> hh = HiveMetastoreHook()
+ >>> hh.drop_partitions(db='airflow', table_name='static_babynames', part_vals="['2020-05-01']")
+ True
+ """
+ if self.table_exists(table_name, db):
+ with self.metastore as client:
+ return client.drop_partition(db, table_name, part_vals, delete_data)
+ else:
+ self.log.info("Table %s.%s does not exist!" % (db, table_name))
Review comment:
Got it. i can add that.
----------------------------------------------------------------
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] Acehaidrey commented on pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
Acehaidrey commented on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-650518730
@vanka56 loook at the error logs
```
There were some pylint errors. Exiting
###########################################################################################
EXITING /opt/airflow/scripts/ci/in_container/run_pylint_tests.sh WITH STATUS CODE 1
###########################################################################################
************* Module tests.providers.apache.hive.hooks.test_hive
tests/providers/apache/hive/hooks/test_hive.py:561:0: C0301: Line too long (115/110) (line-too-long)
tests/providers/apache/hive/hooks/test_hive.py:561:8: W1503: Redundant use of assertTrue with constant value True (redundant-unittest-assert)
```
----------------------------------------------------------------
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] Acehaidrey commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
Acehaidrey commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r443822020
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -775,6 +775,20 @@ def table_exists(self, table_name, db='default'):
except Exception: # pylint: disable=broad-except
return False
+ def drop_partitions(self, table_name, part_vals, delete_data=False, db='default'):
+ """
+ Drop partitions matching param_names input
+ >>> hh = HiveMetastoreHook()
+ >>> hh.drop_partitions(db='airflow', table_name='static_babynames', part_vals="['2020-05-01']")
+ True
+ """
+ if self.table_exists(table_name, db):
+ with self.metastore as client:
+ return client.drop_partition(db, table_name, part_vals, delete_data)
+ else:
+ self.log.info("Table %s.%s does not exist!" % (db, table_name))
Review comment:
maybe make a log message about the partition existing. do we want to check that too before we try to drop the partition? There is a function in the hook for this too.
----------------------------------------------------------------
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] Acehaidrey commented on pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
Acehaidrey commented on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-650947153
@vanka56 ```airflow/providers/apache/hive/hooks/hive.py:789:30: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
airflow/providers/apache/hive/hooks/hive.py:793:26: W1202: Use lazy % formatting in logging functions (logging-format-interpolation)
``` still
----------------------------------------------------------------
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] turbaszek merged pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
turbaszek merged pull request #9472:
URL: https://github.com/apache/airflow/pull/9472
----------------------------------------------------------------
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] Acehaidrey commented on pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
Acehaidrey commented on pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#issuecomment-652627140
@vanka56
```
Trim Trailing Whitespace..........................................................................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook
```
can you fix that
----------------------------------------------------------------
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] turbaszek commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #9472:
URL: https://github.com/apache/airflow/pull/9472#discussion_r449756748
##########
File path: tests/providers/apache/hive/hooks/test_hive.py
##########
@@ -383,6 +383,10 @@ def test_table_exists(self):
self.hook.table_exists(str(random.randint(1, 10000)))
)
+ def test_drop_partition(self):
+ self.assertTrue(self.hook.drop_partitions(self.table, db=self.database,
+ part_vals=[DEFAULT_DATE_DS]))
+
Review comment:
Side effects in tests are not a good practise and Airflow already has number of flaky tests. So I would be in favor of using mocking.
----------------------------------------------------------------
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