Add missing fields in DynamoDBToS3Operator system test by utkarsharma2 · Pull Request #31197 · apache/airflow

utkarsharma2 · 2023-05-11T03:34:26Z

There was a missing s3_key_prefix in the system test for DynamoDBToS3Operator and the export_time date format was not correct.

uranusjr · 2023-05-11T03:47:51Z

tests/system/providers/amazon/aws/example_dynamodb_to_s3.py

        file_size=1000,
        s3_bucket_name=bucket_name,
-        export_time=datetime(year=2023, month=4, day=10),
+        export_time=datetime.now(),


Is this change significant?

Yes, because in system tests we just want to make sure the DynamoDBToS3Operator is successfully able to call export_table_to_point_in_time with the minimum time window. So the exported data is small. But I'm open to any suggestions.

cc: @ferruzzi @o-nikolas @vincbeck

As long as we don't get weird API behaviour with a relatively instantaneous export_time seems fine to me. I'm assuming you've tested this?

Yes, I have tested it on local and I have been developing using datetime.now() only, no issues so far.

ferruzzi · 2023-05-11T05:57:23Z

tests/system/providers/amazon/aws/example_dynamodb_to_s3.py

        s3_bucket_name=bucket_name,
-        export_time=datetime(year=2023, month=4, day=10),
+        export_time=datetime.now(),
+        s3_key_prefix=f"{S3_KEY_PREFIX}-3-",


Why are we hard-coding the -3- part?

~~No reason was just following other examples in the file~~

I think it's good to have it since it isolates this test case from other test cases in the file. Otherwise, there can be some side-effects if two testcase uses the same prefix. WDYT?

I can get behind that reasoning. I didn't see the other two in the github "changes" diff, so I didn't catch the pattern.

o-nikolas · 2023-05-11T18:15:24Z

I re-triggered the tests since the wait for ci images timed out.

ferruzzi

Third time's the charm? :P

ferruzzi · 2023-05-12T21:04:04Z

negative.



ERROR    airflow.executors.debug_executor.DebugExecutor:debug_executor.py:95 Failed to execute task: An error occurred (InvalidExportTimeException) when calling the ExportTableToPointInTime operation: Export Time is invalid.
--
Traceback (most recent call last):
File "/opt/airflow/airflow/executors/debug_executor.py", line 89, in _run_task
ti.run(job_id=ti.job_id, **params)
File "/opt/airflow/airflow/utils/session.py", line 76, in wrapper
return func(*args, session=session, **kwargs)
File "/opt/airflow/airflow/models/taskinstance.py", line 1798, in run
mark_success=mark_success, test_mode=test_mode, job_id=job_id, pool=pool, session=session
File "/opt/airflow/airflow/utils/session.py", line 73, in wrapper
return func(*args, **kwargs)
File "/opt/airflow/airflow/models/taskinstance.py", line 1485, in _run_raw_task
self._execute_task_with_callbacks(context, test_mode)
File "/opt/airflow/airflow/models/taskinstance.py", line 1636, in _execute_task_with_callbacks
result = self._execute_task(context, task_orig)
File "/opt/airflow/airflow/models/taskinstance.py", line 1704, in _execute_task
result = execute_callable(context=context)
File "/opt/airflow/airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py", line 142, in execute
self._export_table_to_point_in_time()
File "/opt/airflow/airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py", line 158, in _export_table_to_point_in_time
ExportFormat=self.export_format,
File "/usr/local/lib/python3.7/site-packages/botocore/client.py", line 530, in _api_call
return self._make_api_call(operation_name, kwargs)
File "/usr/local/lib/python3.7/site-packages/botocore/client.py", line 960, in _make_api_call
raise error_class(parsed_response, operation_name)
botocore.errorfactory.InvalidExportTimeException: An error occurred (InvalidExportTimeException) when calling the ExportTableToPointInTime operation: Export Time is invalid

utkarsharma2 · 2023-05-13T15:16:28Z

@ferruzzi I'm not able to reproduce this on my local below code is working for me. I'm using breeze to run my test which is using docker image - ghcr.io/apache/airflow/main/ci/python3.7:latest

from datetime import datetime

from airflow import DAG
from airflow.providers.amazon.aws.transfers.dynamodb_to_s3 import DynamoDBToS3Operator

with DAG(
    dag_id='example_export_dynamodb',
    schedule_interval=None,
    start_date=datetime(2021, 1, 1),
    tags=['example'],
    catchup=False,
) as dag:
    dynamodb_to_s3_operator = DynamoDBToS3Operator(
        task_id="dynamodb_to_s3",
        dynamodb_table_name="test",
        s3_bucket_name="tmp9",
        file_size=4000,
        export_time=datetime.now(),
        s3_key_prefix="test"
    )

Can you please share the details of environment in which you are running tests? I'll try to reproduce this on my local.

vincbeck · 2023-05-15T14:51:37Z

Our environment is using Breeze in order to run system tests. We are building the last image with breeze ci-image build and then run the test using the command breeze testing tests <filename> --system amazon.

But given the exception I dont think it is question of breeze, the service Dynamodb is complaining that export_time is invalid. My guess is datetime.now() returns in some cases (depending on the timezone) a date in the future and in some cases a date in the present/past. I might recommend to use DateTime.UtcNow() instead with some buffer (e.g. exclude one week maybe?)

potiuk · 2023-05-15T15:04:31Z

Our environment is using Breeze in order to run system tests. We are building the last image with breeze ci-image build and then run the test using the command breeze testing tests <filename> --system amazon.

But given the exception I dont think it is question of breeze, the service Dynamodb is complaining that export_time is invalid. My guess is datetime.now() returns in some cases (depending on the timezone) a date in the future and in some cases a date in the present/past. I might recommend to use DateTime.UtcNow() instead with some buffer (e.g. exclude one week maybe?)

Oh yeah. I've seen it happen in the past.

utkarsharma2 · 2023-05-16T04:21:18Z

@vincbeck I think the datetime.now() return date which is not timezone aware, so for me in India - IST this will always give time which is ahead of UTC and If the export API assumes UTC time, it should have failed for me every time(which it didn't), right? or I'm missing something?

utkarsharma2 · 2023-05-16T04:22:15Z

@vincbeck @potiuk Added a PR - #31307 please review.

Add missing s3_key_prefix

b3d4372

boring-cyborg bot added area:providers area:system-tests provider:amazon AWS/Amazon - related issues labels May 11, 2023

Add correct export date

5c261c7

utkarsharma2 changed the title ~~Add missing s3_key_prefix~~ Add missing DynamoDBToS3Operator system test May 11, 2023

utkarsharma2 mentioned this pull request May 11, 2023

DynamoDBToS3Operator - Add a feature to export the table to a point in time. #31142

Merged

uranusjr reviewed May 11, 2023

View reviewed changes

ferruzzi reviewed May 11, 2023

View reviewed changes

vincbeck approved these changes May 11, 2023

View reviewed changes

ferruzzi approved these changes May 11, 2023

View reviewed changes

o-nikolas approved these changes May 12, 2023

View reviewed changes

o-nikolas merged commit 37d8d62 into apache:main May 12, 2023

utkarsharma2 changed the title ~~Add missing DynamoDBToS3Operator system test~~ Add missing fields in DynamoDBToS3Operator system test May 16, 2023

hussein-awala mentioned this pull request May 16, 2023

DynamoDBToS3Operator - change export_time value to utcnow() in system test #31307

Merged

Comments

Conversation

utkarsharma2 commented May 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uranusjr May 11, 2023

Choose a reason for hiding this comment

Uh oh!

utkarsharma2 May 11, 2023

Choose a reason for hiding this comment

Uh oh!

utkarsharma2 May 11, 2023

Choose a reason for hiding this comment

Uh oh!

o-nikolas May 11, 2023

Choose a reason for hiding this comment

Uh oh!

utkarsharma2 May 11, 2023

Choose a reason for hiding this comment

Uh oh!

ferruzzi May 11, 2023

Choose a reason for hiding this comment

Uh oh!

utkarsharma2 May 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferruzzi May 11, 2023

Choose a reason for hiding this comment

Uh oh!

o-nikolas commented May 11, 2023

Uh oh!

ferruzzi left a comment

Choose a reason for hiding this comment

Uh oh!

ferruzzi commented May 12, 2023

Uh oh!

utkarsharma2 commented May 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincbeck commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

potiuk commented May 15, 2023

Uh oh!

utkarsharma2 commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

utkarsharma2 commented May 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

utkarsharma2 commented May 11, 2023 •

edited

Loading

utkarsharma2 May 11, 2023 •

edited

Loading

utkarsharma2 commented May 13, 2023 •

edited

Loading

vincbeck commented May 15, 2023 •

edited

Loading

utkarsharma2 commented May 16, 2023 •

edited

Loading