You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "slycyberguy (via GitHub)" <gi...@apache.org> on 2024/04/07 02:51:54 UTC
[PR] Add six tests for AWS triggers [airflow]
slycyberguy opened a new pull request, #38810:
URL: https://github.com/apache/airflow/pull/38810
Unit tests added for amazon/aws/triggers: rds, lambda_function, glue_crawler, emr, batch, athena
<!--
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] Add six tests for AWS triggers [airflow]
Posted by "slycyberguy (via GitHub)" <gi...@apache.org>.
slycyberguy commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1555028495
##########
.github/workflows/ci.yml:
##########
Review Comment:
Yes, I see that this PR says the file is deleted. So what steps should I take? I already pushed the addition of the file into the other PR.
--
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] Add six tests for AWS triggers [airflow]
Posted by "slycyberguy (via GitHub)" <gi...@apache.org>.
slycyberguy commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1555019303
##########
.github/workflows/ci.yml:
##########
Review Comment:
That makes sense. I have now added the original file to the PR. Sorry, I'm new to GitHub. At first, I thought the one in your repo wouldn't be affected. Let me know if there are other actions I need to take.
--
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] Add six tests for AWS triggers [airflow]
Posted by "slycyberguy (via GitHub)" <gi...@apache.org>.
slycyberguy commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1554839150
##########
.github/workflows/ci.yml:
##########
Review Comment:
On my local repo, I added CodeCov to the file and I accidentally committed it. So I removed the update.
--
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] Add six tests for AWS triggers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1555005243
##########
.github/workflows/ci.yml:
##########
Review Comment:
So you should bring it back - otherwise no tests are run at all and if we merge it, it will break our CI.
--
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] Add six tests for AWS triggers [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1554824276
##########
.github/workflows/ci.yml:
##########
Review Comment:
Why are you removing this file?
--
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] Add six tests for AWS triggers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1555021343
##########
.github/workflows/ci.yml:
##########
Review Comment:
Still not there
--
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] Add six tests for AWS triggers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1555043528
##########
.github/workflows/ci.yml:
##########
Review Comment:
Restore the file, the usual way.
We generally don't teach git basics, the expectation is that you learn it from some dedicated tutorials.
I am not sure what to advice you as I am not sure what clients and tooling you use but I'd personally use some combination of `git reset HEAD^`, `git add -p`, `git rebase`, `git commit -m`, `git push -f`. But it's up to you and you will have to learn all the basic things ( but also git rebase) in order to contribute.
BTW. Were the tests generated by an AI maybe? They look like generated.
--
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] Add six tests for AWS triggers [airflow]
Posted by "slycyberguy (via GitHub)" <gi...@apache.org>.
slycyberguy commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1555045730
##########
.github/workflows/ci.yml:
##########
Review Comment:
Thanks for the advice. I'll take care of it.
I'm new to unit testing, so I started off with developing unit tests for simpler services. I used AI on a couple to help me get started. Then I kept everything else consistent as I coded the rest.
--
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] Add six tests for AWS triggers [airflow]
Posted by "slycyberguy (via GitHub)" <gi...@apache.org>.
slycyberguy closed pull request #38810: Add six tests for AWS triggers
URL: https://github.com/apache/airflow/pull/38810
--
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] Add six tests for AWS triggers [airflow]
Posted by "slycyberguy (via GitHub)" <gi...@apache.org>.
slycyberguy commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1555025374
##########
.github/workflows/ci.yml:
##########
Review Comment:
I added it to PR #38809. That's where it was deleted. Do I need to add it to this PR as well? Originally, all the unit tests and the ci.yml deletion were on that PR (#38809), but I wanted to separate it into two PRs. So I force pushed the removal of unit tests for aws/triggers from that PR (#38809). Then I created a new branch 'alt' and submitted a new PR (this one #38810) with the trigger tests, but for some reason the commits from my 'main' branch is showing up here.
--
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] Add six tests for AWS triggers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #38810:
URL: https://github.com/apache/airflow/pull/38810#discussion_r1555025952
##########
.github/workflows/ci.yml:
##########
Review Comment:
each PR is separate. You should not remove the .ci.yml in your main. Look at the list of changed files in this PR.
--
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