Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clean up JenkinsJobTriggerOperator #19019

Merged
merged 6 commits into from
Dec 20, 2021
Merged

Clean up JenkinsJobTriggerOperator #19019

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
10d984f
Remove dead code path from JenkinsJobTriggerOperator
xinbinhuang Oct 15, 2021
5d1fd30
Match type to default arg
binh-stripe Oct 15, 2021
7cd5ca7
Remove literal_eval
binh-stripe Oct 15, 2021
c324c11
!fixup Remove ast import
xinbinhuang Oct 17, 2021
cc9f7a7
Remain comp
xinbinhuang Dec 4, 2021
9350d12
Update jenkins_job_trigger.py
xinbinhuang Dec 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 2 additions & 25 deletions airflow/providers/jenkins/operators/jenkins_job_trigger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def __init__(
*,
jenkins_connection_id: str,
job_name: str,
parameters: ParamType = "",
parameters: ParamType = None,
sleep_time: int = 10,
max_try_before_job_appears: int = 10,
allowed_jenkins_states: Optional[Iterable[str]] = None,
Expand All @@ -118,7 +118,7 @@ def __init__(
self.max_try_before_job_appears = max_try_before_job_appears
self.allowed_jenkins_states = list(allowed_jenkins_states) if allowed_jenkins_states else ['SUCCESS']

def build_job(self, jenkins_server: Jenkins, params: ParamType = "") -> Optional[JenkinsRequest]:
def build_job(self, jenkins_server: Jenkins, params: ParamType = None) -> Optional[JenkinsRequest]:
"""
This function makes an API call to Jenkins to trigger a build for 'job_name'
It returned a dict with 2 keys : body and headers.
Expand All @@ -130,15 +130,6 @@ def build_job(self, jenkins_server: Jenkins, params: ParamType = "") -> Optional
:return: Dict containing the response body (key body)
and the headers coming along (headers)
"""
# Since params can be either JSON string, dictionary, or list,
# check type and pass to build_job_url
if params and isinstance(params, str):
params = ast.literal_eval(params)
Copy link
Contributor Author

@xinbinhuang xinbinhuang Oct 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this will cause a small backward incompatibility if a user somehow use a str to represent either dict or list. So I'm happy to leave it as it's. Let me know if you want me keep it.

i.e.

import ast
import json


assert isinstance(ast.literal_eval(json.dumps({"param":1})), dict)
assert isinstance(ast.literal_eval(json.dumps([("param", "1")])), list)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should retain the conversion for backward compatibility

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Keep the compatibility


# We need a None to call the non-parametrized jenkins api end point
if not params:
params = None

request = Request(method='POST', url=jenkins_server.build_job_url(self.job_name, params, None))
return jenkins_request_with_headers(jenkins_server, request)

Expand Down Expand Up @@ -183,20 +174,6 @@ def get_hook(self) -> JenkinsHook:
return JenkinsHook(self.jenkins_connection_id)

def execute(self, context: Mapping[Any, Any]) -> Optional[str]:
if not self.jenkins_connection_id:
self.log.error(
'Please specify the jenkins connection id to use.'
'You must create a Jenkins connection before'
' being able to use this operator'
)
raise AirflowException(
'The jenkins_connection_id parameter is missing, impossible to trigger the job'
)

if not self.job_name:
self.log.error("Please specify the job name to use in the job_name parameter")
raise AirflowException('The job_name parameter is missing,impossible to trigger the job')

self.log.info(
'Triggering the job %s on the jenkins : %s with the parameters : %s',
self.job_name,
Expand Down