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

version_added field in configuration option doesn't work correctly in providers documentation #34005

Closed
1 of 2 tasks
Taragolis opened this issue Sep 1, 2023 · 7 comments · Fixed by #34011
Closed
1 of 2 tasks
Labels
area:core area:providers kind:bug This is a clearly a bug kind:documentation needs-triage label for new issues that we didn't triage yet

Comments

@Taragolis
Copy link
Contributor

Apache Airflow version

2.7.0

What happened

Initial finding: #33960 (comment)

Since Airflow 2.7.0 we have an ability to store configuration options in providers, everything works fine, except field version_added.

The logic around this field expect Airflow version and not Provider version. Any attempt to add in this field any value greater than current version of Airflow (2.7.0 at that moment) will result that configuration option won't rendered in documentation, seem like it not prevented to add this configuration at least airflow config get-value command return expected option.

What you think should happen instead

Various, depend on final solution and decision.

Option 1:
In case if we would not like use this field for providers we might ignore this field in providers configurations.
For Community Providers we could always set it to ~

Option 2:
Dynamically resolve depend on what a source of this configuration, Core/Provider

Option 3:
Add provider_version_added and use for show in which version of provider this configuration added
We could keep version_added if configuration option in provider related to Airflow Version

Option 4:
Suggest you own 😺

How to reproduce

Create configuration option with version_added greater than current version of Airflow, for stable it is 2.7.0 for dev 2.8.0

config:
  aws:
    description: This section contains settings for Amazon Web Services (AWS) integration.
    options:
      session_factory:
        description: |
          Full import path to the class which implements a custom session factory for
          ``boto3.session.Session``. For more details please have a look at
          :ref:`howto/connection:aws:session-factory`.
        default: ~
        example: my_company.aws.MyCustomSessionFactory
        type: string
        version_added: 3.1.1

Operating System

n/a

Versions of Apache Airflow Providers

n/a

Deployment

Other

Deployment details

No response

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@Taragolis Taragolis added kind:bug This is a clearly a bug area:providers area:core needs-triage label for new issues that we didn't triage yet labels Sep 1, 2023
@Taragolis Taragolis changed the title version_added field in configuration option doesn't work correctly in Providers version_added field in configuration option doesn't work correctly in providers Sep 1, 2023
@eladkal
Copy link
Contributor

eladkal commented Sep 1, 2023

I raised it in
#32604 (comment)

@potiuk
Copy link
Member

potiuk commented Sep 1, 2023

I believed (at least ) It's been fixed.

@potiuk
Copy link
Member

potiuk commented Sep 1, 2023

Version added should be "provider_version" when it was added and I was almost 100% sure it's been fixed and that you can put any version there - maybe it's slipped somehow ?

@Taragolis
Copy link
Contributor Author

I check that only IMAP set value greater than 2.7/2.8

config:
imap:
description: "Options for IMAP provider."
options:
ssl_context:
description: |
ssl context to use when using SMTP and IMAP SSL connections. By default, the context is "default"
which sets it to ``ssl.create_default_context()`` which provides the right balance between
compatibility and security, it however requires that certificates in your operating system are
updated and that SMTP/IMAP servers of yours have valid certificates that have corresponding public
keys installed on your machines. You can switch it to "none" if you want to disable checking
of the certificates, but it is not recommended as it allows MITM (man-in-the-middle) attacks
if your infrastructure is not sufficiently secured. It should only be set temporarily while you
are fixing your certificate configuration. This can be typically done by upgrading to newer
version of the operating system you run Airflow components on,by upgrading/refreshing proper
certificates in the OS or by updating certificates for your mail servers.
If you do not set this option explicitly, it will use Airflow "email.ssl_context" configuration,
but if this configuration is not present, it will use "default" value.
type: string
version_added: 3.3.0
example: "default"
default: ~

As result this option not presented into the doc:
https://airflow.apache.org/docs/apache-airflow-providers-imap/stable/configurations-ref.html

I'm not sure for 100% but seems like it might only affect documentation. Because configs generated without any problem

@potiuk
Copy link
Member

potiuk commented Sep 1, 2023

Yes. I think I fixed "generation" but documentation might be wrong.

@Taragolis Taragolis changed the title version_added field in configuration option doesn't work correctly in providers version_added field in configuration option doesn't work correctly in providers documentation Sep 1, 2023
potiuk added a commit to potiuk/airflow that referenced this issue Sep 1, 2023
The `version_added` for packages should use package version, not
Airflow version.

Fixes: apache#34005
@potiuk
Copy link
Member

potiuk commented Sep 1, 2023

Fix in #34011

@potiuk
Copy link
Member

potiuk commented Sep 1, 2023

image

potiuk added a commit that referenced this issue Sep 1, 2023
The `version_added` for packages should use package version, not
Airflow version.

Fixes: #34005
ephraimbuddy pushed a commit that referenced this issue Oct 5, 2023
The `version_added` for packages should use package version, not
Airflow version.

Fixes: #34005
(cherry picked from commit 5595075)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core area:providers kind:bug This is a clearly a bug kind:documentation needs-triage label for new issues that we didn't triage yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants