You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bugraoz93 (via GitHub)" <gi...@apache.org> on 2025/03/05 22:21:23 UTC
[PR] Fix indentation in core conf test [airflow]
bugraoz93 opened a new pull request, #47424:
URL: https://github.com/apache/airflow/pull/47424
Fix indentation in core conf test
<!--
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] Fix indentation in core conf test [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham merged PR #47424:
URL: https://github.com/apache/airflow/pull/47424
--
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] Fix indentation in core conf test [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #47424:
URL: https://github.com/apache/airflow/pull/47424#discussion_r1982376778
##########
tests/core/test_configuration.py:
##########
@@ -641,11 +641,11 @@ def test_auth_backends_adds_session(self):
def test_command_from_env(self):
test_cmdenv_config = """[testcmdenv]
-itsacommand = NOT OK
-notacommand = OK
-"""
+ itsacommand=NOT OK
+ notacommand=OK
+ """
Review Comment:
Thanks for your time! I was going to commit to your suggestions but saw this was missing. I just saw this now you have posted :) That's why I changed it and pushed them in a single commit.
--
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] Fix indentation in core conf test [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #47424:
URL: https://github.com/apache/airflow/pull/47424#discussion_r1982374149
##########
tests/core/test_configuration.py:
##########
@@ -641,9 +641,9 @@ def test_auth_backends_adds_session(self):
def test_command_from_env(self):
test_cmdenv_config = """[testcmdenv]
-itsacommand = NOT OK
-notacommand = OK
-"""
+ itsacommand = NOT OK
Review Comment:
Yes, my bad! I think my brain is exhausted today :) Calling the day off. I applied the suggestions and escaped the first empty line with `\`.
--
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] Fix indentation in core conf test [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #47424:
URL: https://github.com/apache/airflow/pull/47424#discussion_r1982440839
##########
tests/core/test_configuration.py:
##########
@@ -641,9 +641,9 @@ def test_auth_backends_adds_session(self):
def test_command_from_env(self):
test_cmdenv_config = """[testcmdenv]
-itsacommand = NOT OK
-notacommand = OK
-"""
+ itsacommand = NOT OK
Review Comment:
haha no worries!
--
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] Fix indentation in core conf test [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #47424:
URL: https://github.com/apache/airflow/pull/47424#discussion_r1982370115
##########
tests/core/test_configuration.py:
##########
@@ -641,11 +641,11 @@ def test_auth_backends_adds_session(self):
def test_command_from_env(self):
test_cmdenv_config = """[testcmdenv]
-itsacommand = NOT OK
-notacommand = OK
-"""
+ itsacommand=NOT OK
+ notacommand=OK
+ """
Review Comment:
```suggestion
""")
```
Oops, almost missed this part :)
--
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] Fix indentation in core conf test [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #47424:
URL: https://github.com/apache/airflow/pull/47424#discussion_r1982357204
##########
tests/core/test_configuration.py:
##########
@@ -641,11 +641,11 @@ def test_auth_backends_adds_session(self):
def test_command_from_env(self):
test_cmdenv_config = """[testcmdenv]
-itsacommand = NOT OK
-notacommand = OK
-"""
+ itsacommand=NOT OK
+ notacommand=OK
+ """
test_cmdenv_conf = AirflowConfigParser()
- test_cmdenv_conf.read_string(test_cmdenv_config)
+ test_cmdenv_conf.read_string(textwrap.dedent(test_cmdenv_config))
Review Comment:
```suggestion
test_cmdenv_conf.read_string(test_cmdenv_config)
```
That should do it.
##########
tests/core/test_configuration.py:
##########
@@ -641,11 +641,11 @@ def test_auth_backends_adds_session(self):
def test_command_from_env(self):
test_cmdenv_config = """[testcmdenv]
Review Comment:
```suggestion
test_cmdenv_config = testwrap.dedent("""
[testcmdenv]
```
--
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] Fix indentation in core conf test [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #47424:
URL: https://github.com/apache/airflow/pull/47424#discussion_r1982282368
##########
tests/core/test_configuration.py:
##########
@@ -641,9 +641,9 @@ def test_auth_backends_adds_session(self):
def test_command_from_env(self):
test_cmdenv_config = """[testcmdenv]
-itsacommand = NOT OK
-notacommand = OK
-"""
+ itsacommand = NOT OK
Review Comment:
This changes the actual string though. It may still "work", but it's also not how ini is typically... Probably better to use dedent.
--
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] Fix indentation in core conf test [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #47424:
URL: https://github.com/apache/airflow/pull/47424#discussion_r1982356726
##########
tests/core/test_configuration.py:
##########
@@ -641,9 +641,9 @@ def test_auth_backends_adds_session(self):
def test_command_from_env(self):
test_cmdenv_config = """[testcmdenv]
-itsacommand = NOT OK
-notacommand = OK
-"""
+ itsacommand = NOT OK
Review Comment:
How you added dedent doesn't actually change the string :)
dedent removes "common" leading whitepspace. But your first line has none, so nothing gets removed.
--
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] Fix indentation in core conf test [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #47424:
URL: https://github.com/apache/airflow/pull/47424#discussion_r1982298081
##########
tests/core/test_configuration.py:
##########
@@ -641,9 +641,9 @@ def test_auth_backends_adds_session(self):
def test_command_from_env(self):
test_cmdenv_config = """[testcmdenv]
-itsacommand = NOT OK
-notacommand = OK
-"""
+ itsacommand = NOT OK
Review Comment:
Thanks for the quick review! I thought in any indentation, it would be a string in the Python context. In this context, even spaces between values do not fit into the ini file or any source file, updated that too since dedent won't update that part.
--
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