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

Deprecation of operators should be prominently highlighted instead of the one line remark in the documentation #41532

Closed
1 of 2 tasks
peter-gergely-horvath opened this issue Aug 16, 2024 · 22 comments · Fixed by #43909
Assignees

Comments

@peter-gergely-horvath
Copy link

What do you see as an issue?

While working with GCP BigQuery operators, I have noticed that some of the operators are deprecated. As the message was not prominent enough, first I failed to notice it, coded some of my DAGs with the old operators and then needed to re-write them so as to avoid deprecated functions. This is not great when it comes to documentation UX.

For example: https://airflow.apache.org/docs/apache-airflow-providers-google/stable/_api/airflow/providers/google/cloud/operators/bigquery/index.html#airflow.providers.google.cloud.operators.bigquery.BigQueryExecuteQueryOperator

Unless you explicitly double check or look for the deprecation notice, it is very easy to overlook it.

image

Solving the problem

I would propose adding the deprecation notice more prominently and changing the title background for additional highlighting.
If we know the operator is deprecated, all it takes is a little bit of CSS magic to make it absolutely stand out, say

  • Striking the name through
  • Different background color
  • Explicit statement of deprecation in the header

I imagine something like this would be good (at least it is hard to miss the point 😊). Again, nothing more than a few lines of CSS should do the trick:

image

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@peter-gergely-horvath peter-gergely-horvath added kind:bug This is a clearly a bug kind:documentation needs-triage label for new issues that we didn't triage yet labels Aug 16, 2024
@kaxil
Copy link
Member

kaxil commented Aug 16, 2024

I like your proposed solution @peter-gergely-horvath , would you like to help out with the PR? Might just be a change in this file:

https://github.com/apache/airflow/blob/main/docs/sphinx_design/static/custom.css

@sunank200 sunank200 added type:doc-only Changelog: Doc Only pending-response and removed needs-triage label for new issues that we didn't triage yet labels Aug 18, 2024
@geraj1010
Copy link
Contributor

@kaxil I'd be happy to take this on, if the issue is still open.

@potiuk
Copy link
Member

potiuk commented Aug 21, 2024

Assigned you

@peter-gergely-horvath
Copy link
Author

peter-gergely-horvath commented Sep 2, 2024

I like your proposed solution @peter-gergely-horvath , would you like to help out with the PR? Might just be a change in this file:

https://github.com/apache/airflow/blob/main/docs/sphinx_design/static/custom.css

Sorry guys, I only hacked the rendered page DOM inside the browser's dev tools to show what I mean here on a screenshot. I don't think I understand how the doc pages are generated.

@RNHTTR
Copy link
Contributor

RNHTTR commented Sep 11, 2024

I like your proposed solution @peter-gergely-horvath , would you like to help out with the PR? Might just be a change in this file:
https://github.com/apache/airflow/blob/main/docs/sphinx_design/static/custom.css

Sorry guys, I only hacked the rendered page DOM inside the browser's dev tools to show what I mean here on a screenshot. I don't think I understand how the doc pages are generated.

Here's the README for building docs and editing docs

Copy link

This issue has been automatically marked as stale because it has been open for 14 days with no response from the author. It will be closed in next 7 days if no further activity occurs from the issue author.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Sep 26, 2024
@peter-gergely-horvath
Copy link
Author

peter-gergely-horvath commented Sep 27, 2024

The task is now picked up by geraj1010. I look forward to seeing the new design. :)

@github-actions github-actions bot removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Sep 28, 2024
@ferruzzi
Copy link
Contributor

@geraj1010 Are you still working on this one?

@geraj1010
Copy link
Contributor

@ferruzzi Yes I am. I recently just regained access to my account, so I have been delayed. Happy to collaborate on this.

@omkar-foss
Copy link
Collaborator

Hi, as discussed on this slack thread, there are a few Sphinx directives that can be explored that help do this without direct CSS changes:

I suppose any that looks fine and catches the user's attention should be good enough.

cc: @ashb (thanks for the suggestion)

Copy link

This issue has been automatically marked as stale because it has been open for 14 days with no response from the author. It will be closed in next 7 days if no further activity occurs from the issue author.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Oct 31, 2024
@geraj1010
Copy link
Contributor

Greetings, I have been dragging my feet on this issue, but I am starting to look into it now. I think the attention or warning directive fits for this, as mentioned by @omkar-foss.

geraj1010 added a commit to geraj1010/airflow that referenced this issue Nov 7, 2024
@github-actions github-actions bot removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Nov 8, 2024
geraj1010 added a commit to geraj1010/airflow that referenced this issue Nov 8, 2024
geraj1010 added a commit to geraj1010/airflow that referenced this issue Nov 8, 2024
geraj1010 added a commit to geraj1010/airflow that referenced this issue Nov 10, 2024
@geraj1010
Copy link
Contributor

@peter-gergely-horvath I was able to add some additional highlighting to show that the operator is deprecated. It's not exactly like your mockup, but at least it stands out more than just plain text.

image

Thoughts?

@potiuk
Copy link
Member

potiuk commented Nov 11, 2024

Looks good to me.

@geraj1010
Copy link
Contributor

Thank you @potiuk.

@ferruzzi
Copy link
Contributor

I like it too. Definitely an improvement.

@peter-gergely-horvath
Copy link
Author

I would love to see title colour change and strike-through, but if that is difficult to implement, I think this is quite fine. :)

@kaxil
Copy link
Member

kaxil commented Nov 12, 2024

And it can be iterative too: this is already a good improvement :)

@geraj1010
Copy link
Contributor

I would love to see title colour change and strike-through, but if that is difficult to implement, I think this is quite fine. :)

Unfortunately, I think that would be difficult to implement. From what I understand, the documentation for the operators (and sensors) in the Python API section are gathered via Sphinx's AutoAPI extension at document generation runtime. This means that all styling is done on-the-fly and the index.rst files are not directly accessible until after rendering (in a phantom folder called _api).

I was thinking maybe we could achieve the original desired styling, by customizing the AutoAPI extension to look for any classes with the @deprecated decorator and apply a special custom ..py:class directive instead of the normal one. I think that would be very neat, but also a lot more work.

Thank you for the comment :)

@omkar-foss
Copy link
Collaborator

@geraj1010 it's looking nice & catchy! The user will definitely notice it while going through the doc I suppose, great work :)

Copy link

This issue has been automatically marked as stale because it has been open for 14 days with no response from the author. It will be closed in next 7 days if no further activity occurs from the issue author.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Nov 28, 2024
@omkar-foss
Copy link
Collaborator

PR #43909 for this issue is active and approved, but just needs a rebase (this issue isn't necessarily stale as such).

@omkar-foss omkar-foss removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Nov 28, 2024
kaxil pushed a commit to geraj1010/airflow that referenced this issue Nov 29, 2024
@kaxil kaxil closed this as completed in 840018a Nov 29, 2024
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this issue Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants