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/22 10:34:27 UTC
[GitHub] [airflow] SasanAhmadi opened a new pull request #22421: Jenkins sensor
SasanAhmadi opened a new pull request #22421:
URL: https://github.com/apache/airflow/pull/22421
The purpose of this PR is to add a new dedicated sensor for jenkins. This contains the sensor file and a test for it.
closes: #22417
related: #22417
--
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] eladkal commented on a change in pull request #22421: Jenkins sensor
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832237510
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,86 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
+
+import jenkins
+from jenkins import JenkinsException
+
+from airflow.exceptions import AirflowException
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+class JenkinsSensor(BaseSensorOperator):
+ """
+ Monitor a jenkins job and pass when it is finished building. This is regardless of the build outcome.
+ This sensor depend on python-jenkins library,
+ version >= 0.4.15 to communicate with jenkins server.
+ You'll also need to configure a Jenkins connection in the connections screen.
+
+ :param jenkins_connection_id: The jenkins connection to use for this job
+ :param job_name: The name of the job to check
+ :param build_number: Build number to check - if None, the latest build will be used
+ """
+
+ def __init__(
+ self,
+ *,
+ jenkins_connection_id: str,
+ job_name: str,
+ build_number: Optional[int] = None,
+ **kwargs,
+ ):
+ super().__init__(**kwargs)
+ self.job_name = job_name
+ self.build_number = build_number
+ self.jenkins_connection_id = jenkins_connection_id
+
+ def get_hook(self) -> JenkinsHook:
+ """Instantiate jenkins hook"""
+ return JenkinsHook(self.jenkins_connection_id)
+
+ def poke(self, context: 'Context') -> bool:
+ try:
+ self.log.info(f"Poking jenkins job {self.job_name}")
+
+ jenkins_server = self.get_hook().get_jenkins_server()
+ if not self.build_number:
+ job_info = jenkins_server.get_job_info(self.job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ self.log.info(f"Build number not specified, getting latest build info from Jenkins")
+ else:
+ build_number_to_check = self.build_number
+
+ self.log.info(f"Checking build number: #{build_number_to_check}")
+
+ build_info = jenkins_server.get_build_info(self.job_name, build_number_to_check)
+ if build_info['building']:
+ self.log.info(f"Build #{build_number_to_check} is still building")
+ return False
+ else:
+ self.log.info(f"Build #{build_number_to_check} is finished")
+ return True
Review comment:
this logic should not be in the sensor code.
The hook should have a general function that returns a build status.
Then the sensor code should be something like:
```
def poke(self, context: 'Context') -> bool:
hook = ...
if hook.get_build_status(...) = 'building'
return False
else:
return True
```
--
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] eladkal commented on a change in pull request #22421: Jenkins sensor
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832237510
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,86 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
+
+import jenkins
+from jenkins import JenkinsException
+
+from airflow.exceptions import AirflowException
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+class JenkinsSensor(BaseSensorOperator):
+ """
+ Monitor a jenkins job and pass when it is finished building. This is regardless of the build outcome.
+ This sensor depend on python-jenkins library,
+ version >= 0.4.15 to communicate with jenkins server.
+ You'll also need to configure a Jenkins connection in the connections screen.
+
+ :param jenkins_connection_id: The jenkins connection to use for this job
+ :param job_name: The name of the job to check
+ :param build_number: Build number to check - if None, the latest build will be used
+ """
+
+ def __init__(
+ self,
+ *,
+ jenkins_connection_id: str,
+ job_name: str,
+ build_number: Optional[int] = None,
+ **kwargs,
+ ):
+ super().__init__(**kwargs)
+ self.job_name = job_name
+ self.build_number = build_number
+ self.jenkins_connection_id = jenkins_connection_id
+
+ def get_hook(self) -> JenkinsHook:
+ """Instantiate jenkins hook"""
+ return JenkinsHook(self.jenkins_connection_id)
+
+ def poke(self, context: 'Context') -> bool:
+ try:
+ self.log.info(f"Poking jenkins job {self.job_name}")
+
+ jenkins_server = self.get_hook().get_jenkins_server()
+ if not self.build_number:
+ job_info = jenkins_server.get_job_info(self.job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ self.log.info(f"Build number not specified, getting latest build info from Jenkins")
+ else:
+ build_number_to_check = self.build_number
+
+ self.log.info(f"Checking build number: #{build_number_to_check}")
+
+ build_info = jenkins_server.get_build_info(self.job_name, build_number_to_check)
+ if build_info['building']:
+ self.log.info(f"Build #{build_number_to_check} is still building")
+ return False
+ else:
+ self.log.info(f"Build #{build_number_to_check} is finished")
+ return True
Review comment:
this logic should not be in the sensor code.
The hook should have a general function that returns a build status.
Then the sensor code should be something like:
```
def poke(self, context: 'Context') -> bool:
hook = ....
if hook.get_build_status(...)= 'building'
return False
else:
return True
```
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,86 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
+
+import jenkins
+from jenkins import JenkinsException
+
+from airflow.exceptions import AirflowException
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+class JenkinsSensor(BaseSensorOperator):
+ """
+ Monitor a jenkins job and pass when it is finished building. This is regardless of the build outcome.
+ This sensor depend on python-jenkins library,
+ version >= 0.4.15 to communicate with jenkins server.
+ You'll also need to configure a Jenkins connection in the connections screen.
+
+ :param jenkins_connection_id: The jenkins connection to use for this job
+ :param job_name: The name of the job to check
+ :param build_number: Build number to check - if None, the latest build will be used
+ """
+
+ def __init__(
+ self,
+ *,
+ jenkins_connection_id: str,
+ job_name: str,
+ build_number: Optional[int] = None,
+ **kwargs,
+ ):
+ super().__init__(**kwargs)
+ self.job_name = job_name
+ self.build_number = build_number
+ self.jenkins_connection_id = jenkins_connection_id
+
+ def get_hook(self) -> JenkinsHook:
+ """Instantiate jenkins hook"""
+ return JenkinsHook(self.jenkins_connection_id)
+
+ def poke(self, context: 'Context') -> bool:
+ try:
+ self.log.info(f"Poking jenkins job {self.job_name}")
+
+ jenkins_server = self.get_hook().get_jenkins_server()
+ if not self.build_number:
+ job_info = jenkins_server.get_job_info(self.job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ self.log.info(f"Build number not specified, getting latest build info from Jenkins")
+ else:
+ build_number_to_check = self.build_number
+
+ self.log.info(f"Checking build number: #{build_number_to_check}")
+
+ build_info = jenkins_server.get_build_info(self.job_name, build_number_to_check)
+ if build_info['building']:
+ self.log.info(f"Build #{build_number_to_check} is still building")
+ return False
+ else:
+ self.log.info(f"Build #{build_number_to_check} is finished")
+ return True
Review comment:
this logic should not be in the sensor code.
The hook should have a general function that returns a build status.
Then the sensor code should be something like:
```
def poke(self, context: 'Context') -> bool:
hook = ...
if hook.get_build_status(...)= 'building'
return False
else:
return True
```
--
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] eladkal merged pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
eladkal merged pull request #22421:
URL: https://github.com/apache/airflow/pull/22421
--
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] SasanAhmadi commented on a change in pull request #22421: Add JenkinsSensor
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832556489
##########
File path: airflow/providers/jenkins/hooks/jenkins.py
##########
@@ -46,3 +47,24 @@ def __init__(self, conn_id: str = default_conn_name) -> None:
def get_jenkins_server(self) -> jenkins.Jenkins:
"""Get jenkins server"""
return self.jenkins_server
+
+ def get_build_building_state(self, job_name: str, build_number: int) -> bool:
Review comment:
definitely, I am adding one shortly.
--
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] SasanAhmadi commented on pull request #22421: Add JenkinsSensor
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#issuecomment-1075564601
thank you for your swift responses @eladkal 🙏
modified the code, ready for review
--
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] eladkal commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832654251
##########
File path: tests/providers/jenkins/sensors/test_jenkins.py
##########
@@ -0,0 +1,67 @@
+#
+# 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.
+
+import unittest
+from unittest.mock import Mock, patch
+from xmlrpc.client import boolean
+
+import jenkins
+import pytest
+from parameterized import parameterized
+
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.providers.jenkins.sensors.jenkins import JenkinsBuildSensor
+
+class TestJenkinsBuildSensor(unittest.TestCase):
+ @parameterized.expand(
+ [
+ (1,False,),
+ (None, True,),
+ (3, True,),
+ ]
+ )
+ def test_pole(self, _, build_number, build_state):
Review comment:
Did you mean `test_poke`?
--
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] eladkal commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833605402
##########
File path: airflow/providers/jenkins/provider.yaml
##########
@@ -51,6 +51,11 @@ hooks:
python-modules:
- airflow.providers.jenkins.hooks.jenkins
+sensors:
+ - integration-name: Jenkins
+ python-modules:
+ - 'airflow.providers.jenkins.sensors.jenkins_build'
Review comment:
```suggestion
- 'airflow.providers.jenkins.sensors.jenkins'
```
--
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] SasanAhmadi commented on a change in pull request #22421: Add JenkinsSensor
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832558077
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,64 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
+
+import jenkins
+from jenkins import JenkinsException
+
+from airflow.exceptions import AirflowException
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+class JenkinsSensor(BaseSensorOperator):
Review comment:
That is actually makes a lot of sense, perfect naming for the purpose
applied
--
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] SasanAhmadi commented on pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#issuecomment-1076721816
@eladkal can you confirm everything is fine with the last change? slipped from my eye you've already applied the change
--
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] SasanAhmadi edited a comment on pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi edited a comment on pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#issuecomment-1076721816
@eladkal can you confirm everything is fine with the last change? slipped from my eye you've already applied the change
--
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] eladkal commented on a change in pull request #22421: Add JenkinsSensor
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832550436
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,64 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
Review comment:
unused imports?
--
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] SasanAhmadi commented on a change in pull request #22421: Add JenkinsSensor
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832556711
##########
File path: airflow/providers/jenkins/hooks/jenkins.py
##########
@@ -46,3 +47,24 @@ def __init__(self, conn_id: str = default_conn_name) -> None:
def get_jenkins_server(self) -> jenkins.Jenkins:
"""Get jenkins server"""
return self.jenkins_server
+
+ def get_build_building_state(self, job_name: str, build_number: int) -> bool:
+ """Get build building state"""
+ try:
+ if not build_number:
+ self.log.info(f"Build number not specified, getting latest build info from Jenkins")
+ job_info = self.jenkins_server.get_job_info(self.job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ else:
+ build_number_to_check = self.build_number
Review comment:
my bad, fixed
--
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] josh-fell commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833328675
##########
File path: airflow/providers/jenkins/hooks/jenkins.py
##########
@@ -46,3 +49,20 @@ def __init__(self, conn_id: str = default_conn_name) -> None:
def get_jenkins_server(self) -> jenkins.Jenkins:
"""Get jenkins server"""
return self.jenkins_server
+
+ def get_build_building_state(self, job_name: str, build_number: Optional[int]) -> bool:
+ """Get build building state"""
+ try:
+ if not build_number:
+ self.log.info("Build number not specified, getting latest build info from Jenkins")
+ job_info = self.jenkins_server.get_job_info(job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ else:
+ build_number_to_check = build_number
+
+ self.log.info(f"Getting build info for {job_name} build number: #{build_number_to_check}")
Review comment:
```suggestion
self.log.info("Getting build info for %s build number: #%s", job_name, build_number_to_check)
```
We shouldn't mix f-strings within logging to ensure the formatting is not done until the very, very last moment when it's needed. You can read this [quick reference](https://dev.to/izabelakowal/what-is-the-best-string-formatting-technique-for-logging-in-python-d1d) for a little insight as to why.
--
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 #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#issuecomment-1076652942
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
[GitHub] [airflow] SasanAhmadi commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833626712
##########
File path: airflow/providers/jenkins/provider.yaml
##########
@@ -51,6 +51,11 @@ hooks:
python-modules:
- airflow.providers.jenkins.hooks.jenkins
+sensors:
+ - integration-name: Jenkins
+ python-modules:
+ - 'airflow.providers.jenkins.sensors.jenkins_build'
Review comment:
good catch! thanks , applied
--
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] SasanAhmadi commented on a change in pull request #22421: Jenkins sensor
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832470905
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,86 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
+
+import jenkins
+from jenkins import JenkinsException
+
+from airflow.exceptions import AirflowException
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+class JenkinsSensor(BaseSensorOperator):
+ """
+ Monitor a jenkins job and pass when it is finished building. This is regardless of the build outcome.
+ This sensor depend on python-jenkins library,
+ version >= 0.4.15 to communicate with jenkins server.
+ You'll also need to configure a Jenkins connection in the connections screen.
Review comment:
removed the minimum library note
also, removed the obvious fact of connection creation
--
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] eladkal commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833550537
##########
File path: tests/providers/jenkins/sensors/test_jenkins_build.py
##########
@@ -0,0 +1,71 @@
+#
Review comment:
This test file should also be `test_jenkins.py`
--
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] SasanAhmadi commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832674319
##########
File path: tests/providers/jenkins/sensors/test_jenkins.py
##########
@@ -0,0 +1,67 @@
+#
+# 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.
+
+import unittest
+from unittest.mock import Mock, patch
+from xmlrpc.client import boolean
+
+import jenkins
+import pytest
+from parameterized import parameterized
+
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.providers.jenkins.sensors.jenkins import JenkinsBuildSensor
+
+class TestJenkinsBuildSensor(unittest.TestCase):
+ @parameterized.expand(
+ [
+ (1,False,),
+ (None, True,),
+ (3, True,),
+ ]
+ )
+ def test_pole(self, _, build_number, build_state):
Review comment:
typo, fixed
--
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] SasanAhmadi commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833545848
##########
File path: airflow/providers/jenkins/sensors/jenkins_build.py
##########
@@ -0,0 +1,61 @@
+#
+# 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.
+
+from typing import TYPE_CHECKING, Optional
+
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+if TYPE_CHECKING:
+ from airflow.utils.context import Context
+
+
+class JenkinsBuildSensor(BaseSensorOperator):
+ """
+ Monitor a jenkins job and pass when it is finished building. This is regardless of the build outcome.
+ This sensor depend on python-jenkins library,
+
+ :param jenkins_connection_id: The jenkins connection to use for this job
+ :param job_name: The name of the job to check
+ :param build_number: Build number to check - if None, the latest build will be used
+ """
+
+ def __init__(
+ self,
+ *,
+ jenkins_connection_id: str,
+ job_name: str,
+ build_number: Optional[int] = None,
+ **kwargs,
+ ):
+ super().__init__(**kwargs)
+ self.job_name = job_name
+ self.build_number = build_number
+ self.jenkins_connection_id = jenkins_connection_id
+
+ def poke(self, context: 'Context') -> bool:
+ self.log.info(f"Poking jenkins job {self.job_name}")
Review comment:
applied
--
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] SasanAhmadi commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833545217
##########
File path: airflow/providers/jenkins/sensors/jenkins_build.py
##########
@@ -0,0 +1,61 @@
+#
Review comment:
changed the file name to `jenkins.py`
--
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] eladkal commented on a change in pull request #22421: Jenkins sensor
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832548234
##########
File path: airflow/providers/jenkins/hooks/jenkins.py
##########
@@ -46,3 +47,24 @@ def __init__(self, conn_id: str = default_conn_name) -> None:
def get_jenkins_server(self) -> jenkins.Jenkins:
"""Get jenkins server"""
return self.jenkins_server
+
+ def get_build_building_state(self, job_name: str, build_number: int) -> bool:
+ """Get build building state"""
+ try:
+ if not build_number:
+ self.log.info(f"Build number not specified, getting latest build info from Jenkins")
+ job_info = self.jenkins_server.get_job_info(self.job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ else:
+ build_number_to_check = self.build_number
Review comment:
There is no `self.build_number` in the hook. I guess this is a mistake?
##########
File path: airflow/providers/jenkins/hooks/jenkins.py
##########
@@ -46,3 +47,24 @@ def __init__(self, conn_id: str = default_conn_name) -> None:
def get_jenkins_server(self) -> jenkins.Jenkins:
"""Get jenkins server"""
return self.jenkins_server
+
+ def get_build_building_state(self, job_name: str, build_number: int) -> bool:
Review comment:
can you add unit test to the hook function?
##########
File path: airflow/providers/jenkins/hooks/jenkins.py
##########
@@ -46,3 +47,24 @@ def __init__(self, conn_id: str = default_conn_name) -> None:
def get_jenkins_server(self) -> jenkins.Jenkins:
"""Get jenkins server"""
return self.jenkins_server
+
+ def get_build_building_state(self, job_name: str, build_number: int) -> bool:
+ """Get build building state"""
+ try:
+ if not build_number:
+ self.log.info(f"Build number not specified, getting latest build info from Jenkins")
+ job_info = self.jenkins_server.get_job_info(self.job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ else:
+ build_number_to_check = self.build_number
+
+ self.log.info(f"Getting build info for build number: #{build_number_to_check}")
+ build_info = self.jenkins_server.get_build_info(job_name, build_number_to_check)
+ building = build_info['building']
+ return building
+ except jenkins.NotFoundException as err:
+ raise AirflowException(f'Jenkins job status check failed. Final error was: {err.resp.status}')
Review comment:
According to the source code of `get_job_info` & `get_build_info` there is no case of raising `NotFoundException`
This is an internal exception of the functions but once caught the functions raise `JenkinsException`
https://opendev.org/jjb/python-jenkins/src/commit/d1e469649574883bc531d35fb3aee207f45c15a5/jenkins/__init__.py#L632
https://opendev.org/jjb/python-jenkins/src/commit/d1e469649574883bc531d35fb3aee207f45c15a5/jenkins/__init__.py#L452
--
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] josh-fell commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833328675
##########
File path: airflow/providers/jenkins/hooks/jenkins.py
##########
@@ -46,3 +49,20 @@ def __init__(self, conn_id: str = default_conn_name) -> None:
def get_jenkins_server(self) -> jenkins.Jenkins:
"""Get jenkins server"""
return self.jenkins_server
+
+ def get_build_building_state(self, job_name: str, build_number: Optional[int]) -> bool:
+ """Get build building state"""
+ try:
+ if not build_number:
+ self.log.info("Build number not specified, getting latest build info from Jenkins")
+ job_info = self.jenkins_server.get_job_info(job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ else:
+ build_number_to_check = build_number
+
+ self.log.info(f"Getting build info for {job_name} build number: #{build_number_to_check}")
Review comment:
```suggestion
self.log.info("Getting build info for %s build number: #%s", job_name, build_number_to_check)
```
We shouldn't mix f-strings within logging so the formatting is not done until the very, very last moment when it's needed. You can read this [quick reference](https://dev.to/izabelakowal/what-is-the-best-string-formatting-technique-for-logging-in-python-d1d) for a little insight as to why.
##########
File path: airflow/providers/jenkins/sensors/jenkins_build.py
##########
@@ -0,0 +1,61 @@
+#
+# 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.
+
+from typing import TYPE_CHECKING, Optional
+
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+if TYPE_CHECKING:
+ from airflow.utils.context import Context
+
+
+class JenkinsBuildSensor(BaseSensorOperator):
+ """
+ Monitor a jenkins job and pass when it is finished building. This is regardless of the build outcome.
+ This sensor depend on python-jenkins library,
+
+ :param jenkins_connection_id: The jenkins connection to use for this job
+ :param job_name: The name of the job to check
+ :param build_number: Build number to check - if None, the latest build will be used
+ """
+
+ def __init__(
+ self,
+ *,
+ jenkins_connection_id: str,
+ job_name: str,
+ build_number: Optional[int] = None,
+ **kwargs,
+ ):
+ super().__init__(**kwargs)
+ self.job_name = job_name
+ self.build_number = build_number
+ self.jenkins_connection_id = jenkins_connection_id
+
+ def poke(self, context: 'Context') -> bool:
+ self.log.info(f"Poking jenkins job {self.job_name}")
Review comment:
```suggestion
self.log.info("Poking jenkins job %s", self.job_name)
```
##########
File path: tests/providers/jenkins/sensors/test_jenkins_build.py
##########
@@ -0,0 +1,72 @@
+#
+# 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.
+
+import unittest
+from unittest.mock import MagicMock, patch
+
+from parameterized import parameterized
+
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.providers.jenkins.sensors.jenkins_build import JenkinsBuildSensor
+
+
+class TestJenkinsBuildSensor(unittest.TestCase):
+ @parameterized.expand(
+ [
+ (
+ 1,
+ False,
+ ),
+ (
+ None,
+ True,
+ ),
+ (
+ 3,
+ True,
+ ),
+ ]
+ )
+ @patch('jenkins.Jenkins')
+ def test_poke(self, build_number, build_state, mock_jenkins):
+ target_build_number = build_number if build_number else 10
+
+ jenkins_mock = MagicMock()
+ jenkins_mock.get_job_info.return_value = {'lastBuild': {'number': target_build_number}}
+ jenkins_mock.get_build_info.return_value = {
+ 'building': build_state,
+ }
+ mock_jenkins.return_value = jenkins_mock
+
+ with patch.object(JenkinsHook, 'get_connection') as mock_get_connection:
+ mock_get_connection.return_value = MagicMock()
+
+ sensor = JenkinsBuildSensor(
+ dag=None,
+ jenkins_connection_id="fake_jenkins_connection",
+ task_id="sensor_test",
+ job_name="a_job_on_jenkins",
+ build_number=target_build_number,
+ )
+
+ output = sensor.poke(None)
+
+ assert output == (not build_state)
+ assert jenkins_mock.get_job_info.call_count == 0 if build_number else 1
+ assert jenkins_mock.get_build_info.call_count == 1
+ jenkins_mock.get_build_info.assert_called_once_with('a_job_on_jenkins', target_build_number)
Review comment:
```suggestion
jenkins_mock.get_build_info.assert_called_once_with('a_job_on_jenkins', target_build_number)
```
These are redundant in a way and the `assert_called_once_with()` is a better assertion 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
[GitHub] [airflow] eladkal commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833286806
##########
File path: airflow/providers/jenkins/sensors/jenkins_build.py
##########
@@ -0,0 +1,61 @@
+#
Review comment:
The file name should be just `jenkins.py`
So the import will be:
`from airflow.providers.jenkins.sensors.jenkins import JenkinsBuildSensor`
This is the convention (unless special circumstances) for example you can see:
`from airflow.providers.http.sensors.http import HttpSensor`
--
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] SasanAhmadi commented on a change in pull request #22421: Add JenkinsSensor
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832560867
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,64 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
Review comment:
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
[GitHub] [airflow] eladkal commented on pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#issuecomment-1075668837
Tests are failing:
```
==== Providers sqlite: 3 failures ====
tests/providers/jenkins/sensors/test_jenkins.py::TestJenkinsBuildSensor::test_pole_0: TypeError: test_pole() missing 1 required positional argument: 'build_state'
tests/providers/jenkins/sensors/test_jenkins.py::TestJenkinsBuildSensor::test_pole_1: TypeError: test_pole() missing 1 required positional argument: 'build_state'
tests/providers/jenkins/sensors/test_jenkins.py::TestJenkinsBuildSensor::test_pole_2: TypeError: test_pole() missing 1 required positional argument: 'build_state'
```
You also have some static checks failures. I suggest to follow https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst and use pre-commit it will help you to find the static problems before you push the code
--
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] eladkal commented on a change in pull request #22421: Jenkins sensor
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832232224
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,86 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
+
+import jenkins
+from jenkins import JenkinsException
+
+from airflow.exceptions import AirflowException
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+class JenkinsSensor(BaseSensorOperator):
+ """
+ Monitor a jenkins job and pass when it is finished building. This is regardless of the build outcome.
+ This sensor depend on python-jenkins library,
+ version >= 0.4.15 to communicate with jenkins server.
+ You'll also need to configure a Jenkins connection in the connections screen.
Review comment:
Minimum lib version is 1.0.0 so no need to point that
https://github.com/apache/airflow/blob/fa655fe53dcf34a7b581fe3e881d770921dfc7a0/setup.py#L410
As for the 2nd point also - all sensors work with connections so no reason for special mention of this.
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,86 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
+
+import jenkins
+from jenkins import JenkinsException
+
+from airflow.exceptions import AirflowException
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+class JenkinsSensor(BaseSensorOperator):
+ """
+ Monitor a jenkins job and pass when it is finished building. This is regardless of the build outcome.
+ This sensor depend on python-jenkins library,
+ version >= 0.4.15 to communicate with jenkins server.
+ You'll also need to configure a Jenkins connection in the connections screen.
+
+ :param jenkins_connection_id: The jenkins connection to use for this job
+ :param job_name: The name of the job to check
+ :param build_number: Build number to check - if None, the latest build will be used
+ """
+
+ def __init__(
+ self,
+ *,
+ jenkins_connection_id: str,
+ job_name: str,
+ build_number: Optional[int] = None,
+ **kwargs,
+ ):
+ super().__init__(**kwargs)
+ self.job_name = job_name
+ self.build_number = build_number
+ self.jenkins_connection_id = jenkins_connection_id
+
+ def get_hook(self) -> JenkinsHook:
+ """Instantiate jenkins hook"""
+ return JenkinsHook(self.jenkins_connection_id)
+
+ def poke(self, context: 'Context') -> bool:
+ try:
+ self.log.info(f"Poking jenkins job {self.job_name}")
+
+ jenkins_server = self.get_hook().get_jenkins_server()
+ if not self.build_number:
+ job_info = jenkins_server.get_job_info(self.job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ self.log.info(f"Build number not specified, getting latest build info from Jenkins")
+ else:
+ build_number_to_check = self.build_number
+
+ self.log.info(f"Checking build number: #{build_number_to_check}")
+
+ build_info = jenkins_server.get_build_info(self.job_name, build_number_to_check)
+ if build_info['building']:
+ self.log.info(f"Build #{build_number_to_check} is still building")
+ return False
+ else:
+ self.log.info(f"Build #{build_number_to_check} is finished")
+ return True
Review comment:
this logic should not be in the sensor code.
The hook should have a general function that returns a build status.
Then the sensor code should be something like:
```
def poke(self, context: 'Context') -> bool:
if get_build_status(...)= 'building'
return False
else:
return True
```
--
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] eladkal commented on a change in pull request #22421: Add JenkinsSensor
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832549698
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,64 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
+
+import jenkins
+from jenkins import JenkinsException
+
+from airflow.exceptions import AirflowException
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+class JenkinsSensor(BaseSensorOperator):
Review comment:
Should we rename it to `JenkinsBuildSensor`?
This may be more explicit and will be helpful for cases where we will have other sensor in Jenkins (if that make sense)?
--
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] SasanAhmadi edited a comment on pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi edited a comment on pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#issuecomment-1075807192
> <img alt="eladkal" width="40" height="40" src="https://avatars.githubusercontent.com/u/45845474?s=60&v=4">
I have local environment setup, working on this locally, I will push changes and ask for review again
--
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] SasanAhmadi edited a comment on pull request #22421: Add JenkinsSensor
Posted by GitBox <gi...@apache.org>.
SasanAhmadi edited a comment on pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#issuecomment-1075564601
thank you for your swift responses @eladkal 🙏
modified the code, unit test will come shortly
--
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] SasanAhmadi commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833545970
##########
File path: tests/providers/jenkins/sensors/test_jenkins_build.py
##########
@@ -0,0 +1,72 @@
+#
+# 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.
+
+import unittest
+from unittest.mock import MagicMock, patch
+
+from parameterized import parameterized
+
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.providers.jenkins.sensors.jenkins_build import JenkinsBuildSensor
+
+
+class TestJenkinsBuildSensor(unittest.TestCase):
+ @parameterized.expand(
+ [
+ (
+ 1,
+ False,
+ ),
+ (
+ None,
+ True,
+ ),
+ (
+ 3,
+ True,
+ ),
+ ]
+ )
+ @patch('jenkins.Jenkins')
+ def test_poke(self, build_number, build_state, mock_jenkins):
+ target_build_number = build_number if build_number else 10
+
+ jenkins_mock = MagicMock()
+ jenkins_mock.get_job_info.return_value = {'lastBuild': {'number': target_build_number}}
+ jenkins_mock.get_build_info.return_value = {
+ 'building': build_state,
+ }
+ mock_jenkins.return_value = jenkins_mock
+
+ with patch.object(JenkinsHook, 'get_connection') as mock_get_connection:
+ mock_get_connection.return_value = MagicMock()
+
+ sensor = JenkinsBuildSensor(
+ dag=None,
+ jenkins_connection_id="fake_jenkins_connection",
+ task_id="sensor_test",
+ job_name="a_job_on_jenkins",
+ build_number=target_build_number,
+ )
+
+ output = sensor.poke(None)
+
+ assert output == (not build_state)
+ assert jenkins_mock.get_job_info.call_count == 0 if build_number else 1
+ assert jenkins_mock.get_build_info.call_count == 1
+ jenkins_mock.get_build_info.assert_called_once_with('a_job_on_jenkins', target_build_number)
Review comment:
applied
##########
File path: airflow/providers/jenkins/hooks/jenkins.py
##########
@@ -46,3 +49,20 @@ def __init__(self, conn_id: str = default_conn_name) -> None:
def get_jenkins_server(self) -> jenkins.Jenkins:
"""Get jenkins server"""
return self.jenkins_server
+
+ def get_build_building_state(self, job_name: str, build_number: Optional[int]) -> bool:
+ """Get build building state"""
+ try:
+ if not build_number:
+ self.log.info("Build number not specified, getting latest build info from Jenkins")
+ job_info = self.jenkins_server.get_job_info(job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ else:
+ build_number_to_check = build_number
+
+ self.log.info(f"Getting build info for {job_name} build number: #{build_number_to_check}")
Review comment:
applied
thanks for sharing the blog post
--
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] SasanAhmadi commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r833560224
##########
File path: tests/providers/jenkins/sensors/test_jenkins_build.py
##########
@@ -0,0 +1,71 @@
+#
Review comment:
fixed
--
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] SasanAhmadi commented on a change in pull request #22421: Jenkins sensor
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832527875
##########
File path: airflow/providers/jenkins/sensors/jenkins.py
##########
@@ -0,0 +1,86 @@
+#
+# 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.
+
+from typing import Any, Dict, Optional
+from urllib.error import HTTPError, URLError
+
+import jenkins
+from jenkins import JenkinsException
+
+from airflow.exceptions import AirflowException
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.sensors.base import BaseSensorOperator
+
+class JenkinsSensor(BaseSensorOperator):
+ """
+ Monitor a jenkins job and pass when it is finished building. This is regardless of the build outcome.
+ This sensor depend on python-jenkins library,
+ version >= 0.4.15 to communicate with jenkins server.
+ You'll also need to configure a Jenkins connection in the connections screen.
+
+ :param jenkins_connection_id: The jenkins connection to use for this job
+ :param job_name: The name of the job to check
+ :param build_number: Build number to check - if None, the latest build will be used
+ """
+
+ def __init__(
+ self,
+ *,
+ jenkins_connection_id: str,
+ job_name: str,
+ build_number: Optional[int] = None,
+ **kwargs,
+ ):
+ super().__init__(**kwargs)
+ self.job_name = job_name
+ self.build_number = build_number
+ self.jenkins_connection_id = jenkins_connection_id
+
+ def get_hook(self) -> JenkinsHook:
+ """Instantiate jenkins hook"""
+ return JenkinsHook(self.jenkins_connection_id)
+
+ def poke(self, context: 'Context') -> bool:
+ try:
+ self.log.info(f"Poking jenkins job {self.job_name}")
+
+ jenkins_server = self.get_hook().get_jenkins_server()
+ if not self.build_number:
+ job_info = jenkins_server.get_job_info(self.job_name)
+ build_number_to_check = job_info['lastBuild']['number']
+ self.log.info(f"Build number not specified, getting latest build info from Jenkins")
+ else:
+ build_number_to_check = self.build_number
+
+ self.log.info(f"Checking build number: #{build_number_to_check}")
+
+ build_info = jenkins_server.get_build_info(self.job_name, build_number_to_check)
+ if build_info['building']:
+ self.log.info(f"Build #{build_number_to_check} is still building")
+ return False
+ else:
+ self.log.info(f"Build #{build_number_to_check} is finished")
+ return True
Review comment:
moved the logic to the hook
--
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] SasanAhmadi commented on a change in pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on a change in pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#discussion_r832675049
##########
File path: tests/providers/jenkins/sensors/test_jenkins.py
##########
@@ -0,0 +1,67 @@
+#
+# 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.
+
+import unittest
+from unittest.mock import Mock, patch
+from xmlrpc.client import boolean
+
+import jenkins
+import pytest
+from parameterized import parameterized
+
+from airflow.providers.jenkins.hooks.jenkins import JenkinsHook
+from airflow.providers.jenkins.sensors.jenkins import JenkinsBuildSensor
+
+class TestJenkinsBuildSensor(unittest.TestCase):
+ @parameterized.expand(
+ [
+ (1,False,),
+ (None, True,),
+ (3, True,),
+ ]
+ )
+ def test_pole(self, _, build_number, build_state):
Review comment:
no late night commit ends up good 😄
--
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] SasanAhmadi commented on pull request #22421: Add `JenkinsBuildSensor`
Posted by GitBox <gi...@apache.org>.
SasanAhmadi commented on pull request #22421:
URL: https://github.com/apache/airflow/pull/22421#issuecomment-1075807192
> <img alt="eladkal" width="40" height="40" src="https://avatars.githubusercontent.com/u/45845474?s=60&v=4">
I have local environment setup, working on this locally, I will push changes and ask for review again
--
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