You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/03/19 15:24:21 UTC
[GitHub] [airflow] AdamPaslawski opened a new pull request #22377: Sftp Sensor Newer Than Parameter
AdamPaslawski opened a new pull request #22377:
URL: https://github.com/apache/airflow/pull/22377
Add a parameter newer_than to the sftp sensor to allow for specifying a datetime for which the file in the SFTP site should be newer than
--
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
[GitHub] [airflow] potiuk commented on pull request #22377: Sftp Sensor Newer Than Parameter
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22377:
URL: https://github.com/apache/airflow/pull/22377#issuecomment-1074213470
Static checks are failing
--
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
[GitHub] [airflow] potiuk commented on pull request #22377: Sftp Sensor Newer Than Parameter
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22377:
URL: https://github.com/apache/airflow/pull/22377#issuecomment-1076850681
Just see all the other cases. We are following "strict-optional" which means that when you set something to None, it should be Optional[x] - I guesss? But if it does not work, just look at the rest of the code and try to find similar problems and solutions.
--
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
[GitHub] [airflow] potiuk commented on pull request #22377: Sftp Sensor Newer Than Parameter
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #22377:
URL: https://github.com/apache/airflow/pull/22377#issuecomment-1076853617
Note the LOCATION of the error. It is NOT in definition of the field type, but definition of the constructor (from what I see S3KeySensor has the Optional[x] type in constructor and you don't - so that's the mistake you overlooked I guess.
--
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
[GitHub] [airflow] AdamPaslawski commented on pull request #22377: Sftp Sensor Newer Than Parameter
Posted by GitBox <gi...@apache.org>.
AdamPaslawski commented on pull request #22377:
URL: https://github.com/apache/airflow/pull/22377#issuecomment-1073029514
Hello @potiuk , I made the change to convert all dates to UTC as discussed in #21811.
--
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
[GitHub] [airflow] potiuk merged pull request #22377: Sftp Sensor Newer Than Parameter
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #22377:
URL: https://github.com/apache/airflow/pull/22377
--
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
[GitHub] [airflow] AdamPaslawski commented on pull request #22377: Sftp Sensor Newer Than Parameter
Posted by GitBox <gi...@apache.org>.
AdamPaslawski commented on pull request #22377:
URL: https://github.com/apache/airflow/pull/22377#issuecomment-1076801361
> Static checks are failing
Hi @potiuk ,
They are failing because I am providing a default value of None to the newer_than parameter which is not of type DateTime. I think it makes sense to allow for a default of None.
I checked for inspiration in areas of the project like in airflow.providers.amazon.aws.sensors.s3.S3KeySensor where defaults are set as None in the same way for parameters with specific datatypes.
Is there another design pattern I can use to pass the static checks but still allow for a default of None?
--
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
[GitHub] [airflow] AdamPaslawski commented on pull request #22377: Sftp Sensor Newer Than Parameter
Posted by GitBox <gi...@apache.org>.
AdamPaslawski commented on pull request #22377:
URL: https://github.com/apache/airflow/pull/22377#issuecomment-1079062753
@potiuk That makes perfect sense, I really appreciate the help!
Tests and static checks are passing in breeze now.
--
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
[GitHub] [airflow] potiuk edited a comment on pull request #22377: Sftp Sensor Newer Than Parameter
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #22377:
URL: https://github.com/apache/airflow/pull/22377#issuecomment-1076853617
Note the LOCATION of the error. It is NOT in definition of the field type, but definition of the constructor (from what I see S3KeySensor has the Optional[x] type in constructor and you don't - so that's the mistake you overlooked I guess.
(And MyPY correctly pointed it out as a mistake :) ).
--
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
[GitHub] [airflow] github-actions[bot] commented on pull request #22377: Sftp Sensor Newer Than Parameter
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #22377:
URL: https://github.com/apache/airflow/pull/22377#issuecomment-1074214507
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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