You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org> on 2024/12/27 17:28:35 UTC
[PR] More controls for pgbouncer secrets configuration [airflow]
andrii-korotkov-verkada opened a new pull request, #45248:
URL: https://github.com/apache/airflow/pull/45248
closes: #45171
Allow to disable adding default secret mounts for pgbouncer configs as well as metrics exported database url env variable. This can be useful for cases, where the value is retrieved other way, e.g. secrets provider class.
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #45248:
URL: https://github.com/apache/airflow/pull/45248
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564854346
> Tho I might be running into some incompatibility issues with pytest
Just follow the docs. Breeze is best to reproduce the env. You can also use `uv` for local venv. But the docs are complete and explain what to do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org>.
andrii-korotkov-verkada commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2580696816
Do you know why this error might happen?
```
Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run
```
I don't think I control artifact names in the PR, at least not directly...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564853900
> Thanks for the hints, I've naively tried to just run pytest :)
It works as long as you have the same environment.
Those instructions on running tests are explaining how to get the SAME environment to run your pytest command in - in general. To avoid "works for me" (or rather "does not work for me").
Airflow has 700+ dependencies, 3 database in 8 versions, 10 integration, kubernetes and a number of other variations - recreating it all by **just** running pytest is a nice dream (and our vision in the future with testcontainers and other things) - but so far adding tools to make things running in reproducible way is a bit more efficient (as long as you follow it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2585637256
Awesome work, congrats on your first merged pull request! You are invited to check our [Issue Tracker](https://github.com/apache/airflow/issues) for additional contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org>.
andrii-korotkov-verkada commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564836967
I see, it has to be maintainer-approved. Curious if at least some tests can be made to run by default. That can greatly speed up the iteration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org>.
andrii-korotkov-verkada commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2585119317
I've used it in a live environment and it works well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org>.
andrii-korotkov-verkada commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564852291
> Not until your first PR gets merged. This is protection against bots mining bitcoins and roque PRs trying to exploit weaknesses in Github Actions, and it's mandated by the Infra and security team of The ASF.
Ah, I see, that makes sense.
> But you can run exactly the same tests locally -> All tests that are run in CI are runnable locally - that is basic assumptions for our CI system https://github.com/apache/airflow/tree/main/contributing-docs/testing -> you can see more details about all the types of tests we have and how to reproduce them locally 1:1
Thanks for the hints, I've naively tried to just run pytest :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564838730
But you can run exactly the same tests locally -> All tests that are run in CI are runnable locally - that is basic assumptions for our CI system https://github.com/apache/airflow/tree/main/contributing-docs/testing -> you can see more details about all the types of tests we have and how to reproduce them locally 1:1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org>.
andrii-korotkov-verkada commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564853878
Tho I might be running into some incompatibility issues with pytest
```
ERROR: while parsing the following warning configuration:
error::pytest.PytestReturnNotNoneWarning
This error occurred:
Traceback (most recent call last):
File "/Users/andrii.korotkov/.pyenv/versions/3.10.12/lib/python3.10/site-packages/_pytest/config/__init__.py", line 1638, in parse_warning_filter
category: Type[Warning] = _resolve_warning_category(category_)
File "/Users/andrii.korotkov/.pyenv/versions/3.10.12/lib/python3.10/site-packages/_pytest/config/__init__.py", line 1677, in _resolve_warning_category
cat = getattr(m, klass)
File "/Users/andrii.korotkov/.pyenv/versions/3.10.12/lib/python3.10/site-packages/pytest/__init__.py", line 165, in __getattr__
raise AttributeError(f"module {__name__} has no attribute {name}")
AttributeError: module pytest has no attribute PytestReturnNotNoneWarning
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org>.
andrii-korotkov-verkada commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564581895
Also, if this gets merged, how can I release a new chart version?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org>.
andrii-korotkov-verkada commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564581730
I've refined the functionality and added the tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org>.
andrii-korotkov-verkada commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564628297
How do I trigger tests for Helm etc.? Are they automatically triggered after some time or just don't update the checks summary at first?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564838370
> I see, it has to be maintainer-approved. Curious if at least some tests can be made to run by default. That can greatly speed up the iteration.
Not until your first PR gets merged. This is protection against bots mining bitcoins and roque PRs trying to exploit weaknesses in Github Actions, and it's mandated by the Infra and security team of The ASF.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "andrii-korotkov-verkada (via GitHub)" <gi...@apache.org>.
andrii-korotkov-verkada commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2564874289
I've got non-db unit tests, helm tests and k8s tests to pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] More controls for pgbouncer secrets configuration [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #45248:
URL: https://github.com/apache/airflow/pull/45248#issuecomment-2566454261
> I've got non-db unit tests, helm tests and k8s tests to pass when running with breeze.
Always be rebased.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org