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

Broken url for the source code pointing to example_dags under providers #32295

Open
1 of 2 tasks
Adaverse opened this issue Jun 30, 2023 · 15 comments
Open
1 of 2 tasks

Broken url for the source code pointing to example_dags under providers #32295

Adaverse opened this issue Jun 30, 2023 · 15 comments
Assignees

Comments

@Adaverse
Copy link
Contributor

Adaverse commented Jun 30, 2023

What do you see as an issue?

When we click on source code, we get a 404 page not found

image

Upon investigation, we see that the docs of all the packages are built in airflow/providers/<provider_name>/* except example_dags. I suspect it has something to do with the airflow/example_dags where we are excluding it in various places during doc builds (my guess) which is somehow including airflow/providers/<provider_name>/*/example_dags as well. Renaming the folder to something else fixes the broken URL.

Solving the problem

No response

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@Adaverse Adaverse added kind:bug This is a clearly a bug kind:documentation needs-triage label for new issues that we didn't triage yet labels Jun 30, 2023
@Adaverse Adaverse changed the title Broken url for the source code point to example_dags under providers Broken url for the source code pointing to example_dags under providers Jun 30, 2023
@amoghrajesh
Copy link
Contributor

@potiuk if this is an historic issue, it can be fixed in the new post docs script of airflow-site. Wdyt?

@amoghrajesh
Copy link
Contributor

(I can take it up too if no one is working on it)

@potiuk
Copy link
Member

potiuk commented Jul 4, 2023

Sure. It looks like it is. Needs a bit of investigation where it comes from but likely post-processing the HTML pages is indeed something that might be needed

@jscheffl jscheffl removed the needs-triage label for new issues that we didn't triage yet label Jul 4, 2023
@jscheffl
Copy link
Contributor

jscheffl commented Jul 4, 2023

Interesting, the missing source link seems to be not consistent. For Google Cloud Operators and example Cloud Build code in https://airflow.apache.org/docs/apache-airflow-providers-google/stable/operators/cloud/cloud_build.html the links are not broken.

@Adaverse
Copy link
Contributor Author

Adaverse commented Jul 4, 2023

Interesting, the missing source link seems to be not consistent. For Google Cloud Operators and example Cloud Build code in https://airflow.apache.org/docs/apache-airflow-providers-google/stable/operators/cloud/cloud_build.html the links are not broken.

All the source code pointing to tests.system.providers.google.* (i.e system tests) or for any providers are being built during doc builds, but the source code in airflow/providers/<provider_name>/example_dags aren't being built during doc-builds into HTML. Here the problem is only with the name of the folder i.e example_dags. If we rename it to any other name, then it would work.

We are excluding folders with the pattern */example_dags/* in the config of the sphinx. I think there might be the source to this problem.

@amoghrajesh
Copy link
Contributor

@potiuk @Adaverse I think this is a bigger problem and cannot be fixed post the docs are generated. This should be fixed in the sphinx config.

Why are we excluding the example_dags while building the docs?

    exclude_patterns.extend(
        _get_rst_filepath_from_path(f) for f in pathlib.Path(PACKAGE_DIR).glob("**/example_dags")
    )

@potiuk
Copy link
Member

potiuk commented Jul 5, 2023

@potiuk @Adaverse I think this is a bigger problem and cannot be fixed post the docs are generated. This should be fixed in the sphinx config.

Why are we excluding the example_dags while building the docs?

    exclude_patterns.extend(
        _get_rst_filepath_from_path(f) for f in pathlib.Path(PACKAGE_DIR).glob("**/example_dags")
    )

To be honest. I am not sure. This part has always been a bit "trial-and-error" - likely because enabling it causes a number of "index.rst is not part of any TOC". You can remove it and see what efffect it will have. Likely it will need an index to be generated to those.

I do not think we have good set of "reasoning" in the configuration of those docs - in some cases the changes were "simplest way to make the docs build" but without deeper understanding if some of the links there might be missing.

I think generally speaking example_dags are not considered as public API of the providers, they are just used as "embedded examples" in the docs - and you do not need to have the Python AutoAPI generated documentation, but embedding them using "example include" expects them to be there in the autoapi docs.

Just note: example_dags for most cases has been converted to system tests. There are likely few example_dags that are left and possibly, the right approach will be to move them to system tests.

@potiuk
Copy link
Member

potiuk commented Jul 5, 2023

And yes. Our docs needs someone to take care about making the whole build and generation process better. That's clear.

@potiuk
Copy link
Member

potiuk commented Jul 5, 2023

So making any improvements there is most welcome.

@Adaverse
Copy link
Contributor Author

Adaverse commented Jul 5, 2023

I think it's better to migrate leftover example_dags to system tests and parallelly address the build docs issue and the process overall.

@potiuk
Copy link
Member

potiuk commented Jul 5, 2023

likely yes.

@amoghrajesh
Copy link
Contributor

@Adaverse the google system tests are being taken up by @VladaZakharova separately. Any other migrations that you need as part of this issue?

@Adaverse
Copy link
Contributor Author

Yes @amoghrajesh, been thinking to work on Azure system tests improvements. There are two dags in example_dag folder (I'm the culprit here :)). I have started the works on wasb sensor example dag. There is one more called cosmos document sensor. And also other system test like highlighted here --> #31809. Yet to analyze all the other tests. May be we can do the same exercise on Azure system tests to whatever extent possible but first its important to move the example_dag dags to system test. I have started on wasb sensor part. Feel free to take up the other one.

@amoghrajesh
Copy link
Contributor

Thanks @Adaverse.
I will migrate it to sys tests. I will need some help in testing it. I will raise a pR today for this

@Adaverse
Copy link
Contributor Author

Thanks @Adaverse. I will migrate it to sys tests. I will need some help in testing it. I will raise a pR today for this

Sure

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.

4 participants