Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update AWS Connection docs and deprecate some extras #24670

Merged
merged 2 commits into from
Jul 4, 2022

Conversation

Taragolis
Copy link
Contributor

@Taragolis Taragolis commented Jun 26, 2022

Changes

  • Add missing extra parameters in docs
  • Add links to specific boto3, botocore, AWS, etc. documentations
  • Fix some links in code
  • Mark as deprecated (both in docs and Hook it self): aws_account_id, aws_iam_role, external_id, s3_config_file, s3_config_format, profile

Good candidate for future deprecation:

  1. s3_config_file, s3_config_format, profile
    This options migrated from legacy S3Hook 3865836 in Airflow 1.9, and never been documented and well-tested. This use for parse local credentials file in selected formats:
    • boto config files - boto not maintained for many years
    • AWS Shared Credentials file. In current implementation only get parts credentials. If need to specify non default location to AWS Shared Credentials and AWS Config files better to use configuration of botocore.sessionSession. In this case also could use additional abilities such as credential_process, and profiles inheritance.
    • s3cmd - Seems like this utility never been part of Airflow
  2. host might confuse users (actually couple of issues exists in Github) better rename to endpoint_url
  3. session_kwargs - only profile_name could be use as part of this dictionary, other boto3.session.Session arguments stored directly in extra

@ferruzzi @o-nikolas

@@ -83,6 +83,15 @@ def create_session(self) -> boto3.session.Session:
self.extra_config["session_kwargs"],
)
session_kwargs = self.extra_config["session_kwargs"]

if "profile" in self.extra_config and "s3_config_file" not in self.extra_config:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is s3 specific logic being added to the AWS base session class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why initially it added in AWS Hook or why I add this warning?

Answer for fist part: #2532 (comment)
Answer for second part: This parameter exist in documentation since version 1.1.0 without mentioning how it actually works, I've just add warning to inform user.

docs/apache-airflow-providers-amazon/connections/aws.rst Outdated Show resolved Hide resolved
docs/apache-airflow-providers-amazon/connections/aws.rst Outdated Show resolved Hide resolved
docs/apache-airflow-providers-amazon/connections/aws.rst Outdated Show resolved Hide resolved
docs/apache-airflow-providers-amazon/connections/aws.rst Outdated Show resolved Hide resolved
docs/apache-airflow-providers-amazon/connections/aws.rst Outdated Show resolved Hide resolved
@Taragolis
Copy link
Contributor Author

Fix grammar errors and deprecate config from "local credentials file"

Updated docs

image

@potiuk potiuk merged commit c61f86d into apache:main Jul 4, 2022
@Taragolis Taragolis deleted the aws-connection-docs branch August 6, 2022 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants