Skip to content

Json files from S3 downloading as text files #23514

Closed

Description

@kalyani-subbiah

Apache Airflow Provider(s)

amazon

Versions of Apache Airflow Providers

No response

Apache Airflow version

2.3.0 (latest released)

Operating System

Mac OS Mojave 10.14.6

Deployment

Official Apache Airflow Helm Chart

Deployment details

No response

What happened

When I download a json file from S3 using the S3Hook:

filename=s3_hook.download_file(bucket_name=self.source_s3_bucket, key=key, local_path="./data")

The file is being downloaded as a text file starting with airflow_temp_.

What you think should happen instead

It would be nice to have them download as a json file or keep the same filename as in S3. Since it requires additional code to go back and read the file as a dictionary (ast.literal_eval) and there is no guarantee that the json structure is maintained.

How to reproduce

Where s3_conn_id is the Airflow connection and s3_bucket is a bucket on AWS S3.
This is the custom operator class:

from airflow.models.baseoperator import BaseOperator
from airflow.utils.decorators import apply_defaults
from airflow.hooks.S3_hook import S3Hook

import logging
class S3SearchFilingsOperator(BaseOperator):
	"""
	Queries the Datastore API and uploads the processed info as a csv to the S3 bucket. 

	:param source_s3_bucket:          Choose source s3 bucket
	:param source_s3_directory:       Source s3 directory
	:param s3_conn_id:	              S3 Connection ID
	:param destination_s3_bucket:		  S3 Bucket Destination
	"""

	@apply_defaults
	def __init__(
		self,
		source_s3_bucket=None,
		source_s3_directory=True,
		s3_conn_id=True,
		destination_s3_bucket=None,
		destination_s3_directory=None,
		search_terms=[],
		*args, 
		**kwargs) -> None: 
		
		super().__init__(*args, **kwargs)
		
		self.source_s3_bucket = source_s3_bucket
		self.source_s3_directory = source_s3_directory
		self.s3_conn_id = s3_conn_id
		self.destination_s3_bucket = destination_s3_bucket	
		self.destination_s3_directory = destination_s3_directory	

	def execute(self, context):
		"""
		Executes the operator.
		"""
		s3_hook = S3Hook(self.s3_conn_id)
		keys = s3_hook.list_keys(bucket_name=self.source_s3_bucket)
		for key in keys:
			# download file
			filename=s3_hook.download_file(bucket_name=self.source_s3_bucket, key=key, local_path="./data")
			logging.info(filename)
			with open(filename, 'rb') as handle:
				filing = handle.read()
			filing = pickle.loads(filing)
			logging.info(filing.keys())

And this is the dag file:

from keywordSearch.operators.s3_search_filings_operator import S3SearchFilingsOperator

from airflow import DAG
from airflow.utils.dates import days_ago

from datetime import timedelta

# from aws_pull import aws_pull

default_args = {
  "owner" : "airflow",
  "depends_on_past" : False,
  "start_date": days_ago(2),
  "email" : ["airflow@example.com"],
  "email_on_failure" : False,
  "email_on_retry" : False,
  "retries" : 1,
  "retry_delay": timedelta(seconds=30)
}

with DAG("keyword-search-full-load",
  default_args=default_args,
  description="Syntax Keyword Search",
  max_active_runs=1,
  schedule_interval=None) as dag:

  op3 = S3SearchFilingsOperator(
    task_id="s3_search_filings",
    source_s3_bucket="processed-filings",
    source_s3_directory="citations",
    s3_conn_id="Syntax_S3",
    destination_s3_bucket="keywordsearch",
    destination_s3_directory="results",
    dag=dag
  )

op3

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

Activity

  1. boring-cyborg commented on May 5, 2022

    @boring-cyborg

    Thanks for opening your first issue here! Be sure to follow the issue template!

  2. tirkarthi commented on May 6, 2022

    @tirkarthi
    Contributor

    The logic is here where airflow_tmp_ is hardcoded. Since the name is returned I guess you shutil.move to rename it https://docs.python.org/3/library/shutil.html#shutil.move

    with NamedTemporaryFile(dir=local_path, prefix='airflow_tmp_', delete=False) as local_tmp_file:
    s3_obj.download_fileobj(local_tmp_file)

    Change introduced in #6577 . cc: @feluelle

  3. kalyani-subbiah commented on May 6, 2022

    @kalyani-subbiah
    Author

    I see. In my project since I'm downloading a lot of files, the filenames are important to keep track of what the contents are. When it gets downloaded as a temp file with a different name, this content is lost. I will have to delete all the contents of the temp directory for each iteration which is time-consuming.

    One thing that would be nice is if we could specify the file name for the download. Any reason why we're using .download_fileobj over .download_file from boto3? The .download_file function would be much better since a local filename can be specified:
    https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.download_file

  4. feluelle commented on May 7, 2022

    @feluelle
    Member

    Hi @kalyani-subbiah,

    I would say we can change this behavior to use open and write to a real file instead of a temp file as like you said you can properly set the file name incl. ending.

    I can't remember why I initially did it like this. Maybe the use case was only to dump the data and then forget about it, as by default it writes to the temp dir.

    Feel free to open a PR with how you think it should be. But first we should check where this method is used in the code base and then also very importantly consider backwards compatibility.

    Thanks 🙏😊

  5. kalyani-subbiah commented on May 7, 2022

    @kalyani-subbiah
    Author

    Hi @feluelle . Thanks, will look into creating a PR now.

  6. kalyani-subbiah commented on May 11, 2022

    @kalyani-subbiah
    Author

    Hi I just created a PR for this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions