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/11/12 10:09:11 UTC
[GitHub] [airflow] Adityamalik123 opened a new pull request, #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Adityamalik123 opened a new pull request, #27633:
URL: https://github.com/apache/airflow/pull/27633
<!--
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/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.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
[GitHub] [airflow] Adityamalik123 commented on a diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1036407037
##########
airflow/providers/atlassian/jira/provider.yaml:
##########
@@ -22,11 +22,12 @@ description: |
`Atlassian Jira <https://www.atlassian.com/>`__
versions:
+ - 2.0.0
Review Comment:
Got it. Removed this 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] Adityamalik123 commented on a diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1039375129
##########
airflow/providers/atlassian/jira/operators/jira.py:
##########
@@ -76,16 +78,14 @@ def execute(self, context: Context) -> Any:
hook = JiraHook(jira_conn_id=self.jira_conn_id)
resource = hook.client
- # Current Jira-Python SDK (1.0.7) has issue with pickling the jira response.
- # ex: self.xcom_push(context, key='operator_response', value=jira_response)
- # This could potentially throw error if jira_result is not picklable
jira_result = getattr(resource, self.method_name)(**self.jira_method_args)
+
+ output = jira_result.get("id", None) if jira_result is not None else None
+ self.xcom_push(context, key="id", value=output)
+
if self.result_processor:
return self.result_processor(context, jira_result)
return jira_result
-
- except JIRAError as jira_error:
- raise AirflowException(f"Failed to execute jiraOperator, error: {str(jira_error)}")
- except Exception as e:
- raise AirflowException(f"Jira operator error: {str(e)}")
+ except HTTPError as e:
+ raise AirflowException(f"Failed to execute jiraOperator, error: {str(e.response)}")
Review Comment:
> I think we don't really need this? the error from the sdk will simply populate to Airflow and will fail the task.. there is not need to wrap it with Airflow exception
I just thought since we (provider code) are calling the external library, we should log the returned response in case of failure. Without this, I believe It is only gonna log the traceback by default.
But yeah its not needed necessarily and i can remove it if you'd like.
--
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] Adityamalik123 commented on pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on PR #27633:
URL: https://github.com/apache/airflow/pull/27633#issuecomment-1336502257
> > > Lets also add entry in the provider change log explaining this breaking change
> >
> >
> > @eladkal Can i add it in the CHANGELOG.rst under a new version or do i have to comment it here and the release manager adds it?
>
> Yeah. You need to add it. Check opsgenie change log for example as it had similar case of changing sdk
Thanks! I have made the changes related to this. Please re-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] Adityamalik123 commented on pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on PR #27633:
URL: https://github.com/apache/airflow/pull/27633#issuecomment-1331670965
@potiuk @eladkal Apologies for the delay, I'll be pushing up the fixes this week.
--
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] Adityamalik123 commented on pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on PR #27633:
URL: https://github.com/apache/airflow/pull/27633#issuecomment-1336432144
> > > Lets also add entry in the provider change log explaining this breaking change
> >
> >
> > @eladkal Can i add it in the CHANGELOG.rst under a new version or do i have to comment it here and the release manager adds it?
>
> Yeah. You need to add it. Check opsgenie change log for example as it had similar case of changing sdk
Thanks! I have pushed the changes related to this. Please re-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] Adityamalik123 commented on pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on PR #27633:
URL: https://github.com/apache/airflow/pull/27633#issuecomment-1332750243
> Lets also add entry in the provider change log explaining this breaking change
@eladkal Can i add it in the CHANGELOG.rst under a new version or do i have to comment it here and the release manager adds it?
--
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 #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #27633:
URL: https://github.com/apache/airflow/pull/27633#issuecomment-1340923987
Thanks @Adityamalik123
Now it should be easy to add new integrations of atlassian products (service desk or others)
--
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 diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1039320387
##########
airflow/providers/atlassian/jira/operators/jira.py:
##########
@@ -76,16 +78,14 @@ def execute(self, context: Context) -> Any:
hook = JiraHook(jira_conn_id=self.jira_conn_id)
resource = hook.client
- # Current Jira-Python SDK (1.0.7) has issue with pickling the jira response.
- # ex: self.xcom_push(context, key='operator_response', value=jira_response)
- # This could potentially throw error if jira_result is not picklable
jira_result = getattr(resource, self.method_name)(**self.jira_method_args)
+
+ output = jira_result.get("id", None) if jira_result is not None else None
+ self.xcom_push(context, key="id", value=output)
+
if self.result_processor:
return self.result_processor(context, jira_result)
return jira_result
-
- except JIRAError as jira_error:
- raise AirflowException(f"Failed to execute jiraOperator, error: {str(jira_error)}")
- except Exception as e:
- raise AirflowException(f"Jira operator error: {str(e)}")
+ except HTTPError as e:
+ raise AirflowException(f"Failed to execute jiraOperator, error: {str(e.response)}")
Review Comment:
I think we don't really need this?
the error from the sdk will simply populate to Airflow and will fail the task.. there is not need to wrap it with Airflow exception
--
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] Adityamalik123 commented on a diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1039400096
##########
airflow/providers/atlassian/jira/operators/jira.py:
##########
@@ -76,16 +78,14 @@ def execute(self, context: Context) -> Any:
hook = JiraHook(jira_conn_id=self.jira_conn_id)
resource = hook.client
- # Current Jira-Python SDK (1.0.7) has issue with pickling the jira response.
- # ex: self.xcom_push(context, key='operator_response', value=jira_response)
- # This could potentially throw error if jira_result is not picklable
jira_result = getattr(resource, self.method_name)(**self.jira_method_args)
+
+ output = jira_result.get("id", None) if jira_result is not None else None
+ self.xcom_push(context, key="id", value=output)
+
if self.result_processor:
return self.result_processor(context, jira_result)
return jira_result
-
- except JIRAError as jira_error:
- raise AirflowException(f"Failed to execute jiraOperator, error: {str(jira_error)}")
- except Exception as e:
- raise AirflowException(f"Jira operator error: {str(e)}")
+ except HTTPError as e:
+ raise AirflowException(f"Failed to execute jiraOperator, error: {str(e.response)}")
Review Comment:
Sure. I have removed the same.
--
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 #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #27633:
URL: https://github.com/apache/airflow/pull/27633#issuecomment-1328343058
Can we count on rebase/fixes @Adityamalik123 ?
--
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 diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1039378773
##########
airflow/providers/atlassian/jira/operators/jira.py:
##########
@@ -76,16 +78,14 @@ def execute(self, context: Context) -> Any:
hook = JiraHook(jira_conn_id=self.jira_conn_id)
resource = hook.client
- # Current Jira-Python SDK (1.0.7) has issue with pickling the jira response.
- # ex: self.xcom_push(context, key='operator_response', value=jira_response)
- # This could potentially throw error if jira_result is not picklable
jira_result = getattr(resource, self.method_name)(**self.jira_method_args)
+
+ output = jira_result.get("id", None) if jira_result is not None else None
+ self.xcom_push(context, key="id", value=output)
+
if self.result_processor:
return self.result_processor(context, jira_result)
return jira_result
-
- except JIRAError as jira_error:
- raise AirflowException(f"Failed to execute jiraOperator, error: {str(jira_error)}")
- except Exception as e:
- raise AirflowException(f"Jira operator error: {str(e)}")
+ except HTTPError as e:
+ raise AirflowException(f"Failed to execute jiraOperator, error: {str(e.response)}")
Review Comment:
I think it's better without it.
We don't wrap SDK exceptions with AirflowException unless there is a good reason (you can check other operators as well)
--
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 diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1020859178
##########
airflow/providers/atlassian/jira/operators/jira.py:
##########
@@ -76,16 +76,11 @@ def execute(self, context: Context) -> Any:
hook = JiraHook(jira_conn_id=self.jira_conn_id)
resource = hook.client
- # Current Jira-Python SDK (1.0.7) has issue with pickling the jira response.
- # ex: self.xcom_push(context, key='operator_response', value=jira_response)
- # This could potentially throw error if jira_result is not picklable
jira_result = getattr(resource, self.method_name)(**self.jira_method_args)
if self.result_processor:
return self.result_processor(context, jira_result)
return jira_result
- except JIRAError as jira_error:
- raise AirflowException(f"Failed to execute jiraOperator, error: {str(jira_error)}")
except Exception as e:
raise AirflowException(f"Jira operator error: {str(e)}")
Review Comment:
same comment/question
##########
airflow/providers/atlassian/jira/sensors/jira.py:
##########
@@ -96,38 +93,38 @@ def __init__(
if field_checker_func is None:
field_checker_func = self.issue_field_checker
- super().__init__(jira_conn_id=jira_conn_id, result_processor=field_checker_func, **kwargs)
+ super().__init__(
+ jira_conn_id=jira_conn_id, method_name="issue", result_processor=field_checker_func, **kwargs
+ )
def poke(self, context: Context) -> Any:
self.log.info("Jira Sensor checking for change in ticket: %s", self.ticket_id)
self.method_name = "issue"
- self.method_params = {"id": self.ticket_id, "fields": self.field}
+ self.method_params = {"key": self.ticket_id, "fields": self.field}
return JiraSensor.poke(self, context=context)
- def issue_field_checker(self, issue: Issue) -> bool | None:
+ def issue_field_checker(self, jira_result: dict) -> bool | None:
"""Check issue using different conditions to prepare to evaluate sensor."""
result = None
try:
- if issue is not None and self.field is not None and self.expected_value is not None:
+ if jira_result is not None and self.field is not None and self.expected_value is not None:
- field_val = getattr(issue.fields, self.field)
+ field_val = jira_result.get("fields", {}).get(self.field, None)
if field_val is not None:
if isinstance(field_val, list):
result = self.expected_value in field_val
elif isinstance(field_val, str):
result = self.expected_value.lower() == field_val.lower()
- elif isinstance(field_val, Resource) and getattr(field_val, "name"):
- result = self.expected_value.lower() == field_val.name.lower()
+ elif isinstance(field_val, dict) and field_val.get("name", None):
+ result = self.expected_value.lower() == field_val.get("name", "").lower()
else:
self.log.warning(
"Not implemented checker for issue field %s which "
"is neither string nor list nor Jira Resource",
self.field,
)
- except JIRAError as jira_error:
- self.log.error("Jira error while checking with expected value: %s", jira_error)
except Exception:
self.log.exception("Error while checking with expected value %s:", self.expected_value)
Review Comment:
same question
##########
airflow/providers/atlassian/jira/operators/jira.py:
##########
@@ -76,16 +76,11 @@ def execute(self, context: Context) -> Any:
hook = JiraHook(jira_conn_id=self.jira_conn_id)
resource = hook.client
- # Current Jira-Python SDK (1.0.7) has issue with pickling the jira response.
- # ex: self.xcom_push(context, key='operator_response', value=jira_response)
- # This could potentially throw error if jira_result is not picklable
jira_result = getattr(resource, self.method_name)(**self.jira_method_args)
if self.result_processor:
return self.result_processor(context, jira_result)
Review Comment:
If we remove the notes since no longer relevant shouldn't the code change as well?
to my understanding this code is a workaround to address the issue in comments
##########
airflow/providers/atlassian/jira/hooks/jira.py:
##########
@@ -43,48 +43,51 @@ def __init__(self, jira_conn_id: str = default_conn_name, proxies: Any | None =
super().__init__()
self.jira_conn_id = jira_conn_id
self.proxies = proxies
- self.client: JIRA | None = None
+ self.client: Jira | None = None
self.get_conn()
- def get_conn(self) -> JIRA:
+ def get_conn(self) -> Jira:
if not self.client:
self.log.debug("Creating Jira client for conn_id: %s", self.jira_conn_id)
- get_server_info = True
- validate = True
- extra_options = {}
+ verify = True
if not self.jira_conn_id:
raise AirflowException("Failed to create jira client. no jira_conn_id provided")
conn = self.get_connection(self.jira_conn_id)
if conn.extra is not None:
extra_options = conn.extra_dejson
# only required attributes are taken for now,
- # more can be added ex: async, logging, max_retries
+ # more can be added ex: timeout, cloud, session
# verify
if "verify" in extra_options and extra_options["verify"].lower() == "false":
- extra_options["verify"] = False
+ verify = False
# validate
- if "validate" in extra_options and extra_options["validate"].lower() == "false":
- validate = False
-
- if "get_server_info" in extra_options and extra_options["get_server_info"].lower() == "false":
- get_server_info = False
+ if "validate" in extra_options:
+ warnings.warn(
+ "Passing 'validate' in the connection is no longer supported.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+
+ if "get_server_info" in extra_options:
+ warnings.warn(
+ "Passing 'get_server_info' in the connection is no longer supported.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
Review Comment:
I think we don't really need this warnings?
we are changing SDKs.. parameters of the previous SDK can just cause confusion.
If users didn't change their usage of the hook nothing won't work anyway.
##########
airflow/providers/atlassian/jira/provider.yaml:
##########
@@ -22,11 +22,12 @@ description: |
`Atlassian Jira <https://www.atlassian.com/>`__
versions:
+ - 2.0.0
Review Comment:
no need for this here. the release manager decide the version upon release
##########
airflow/providers/atlassian/jira/hooks/jira.py:
##########
@@ -43,48 +43,51 @@ def __init__(self, jira_conn_id: str = default_conn_name, proxies: Any | None =
super().__init__()
self.jira_conn_id = jira_conn_id
self.proxies = proxies
- self.client: JIRA | None = None
+ self.client: Jira | None = None
self.get_conn()
- def get_conn(self) -> JIRA:
+ def get_conn(self) -> Jira:
if not self.client:
self.log.debug("Creating Jira client for conn_id: %s", self.jira_conn_id)
- get_server_info = True
- validate = True
- extra_options = {}
+ verify = True
if not self.jira_conn_id:
raise AirflowException("Failed to create jira client. no jira_conn_id provided")
conn = self.get_connection(self.jira_conn_id)
if conn.extra is not None:
extra_options = conn.extra_dejson
# only required attributes are taken for now,
- # more can be added ex: async, logging, max_retries
+ # more can be added ex: timeout, cloud, session
# verify
if "verify" in extra_options and extra_options["verify"].lower() == "false":
- extra_options["verify"] = False
+ verify = False
# validate
- if "validate" in extra_options and extra_options["validate"].lower() == "false":
- validate = False
-
- if "get_server_info" in extra_options and extra_options["get_server_info"].lower() == "false":
- get_server_info = False
+ if "validate" in extra_options:
+ warnings.warn(
+ "Passing 'validate' in the connection is no longer supported.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+
+ if "get_server_info" in extra_options:
+ warnings.warn(
+ "Passing 'get_server_info' in the connection is no longer supported.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
try:
- self.client = JIRA(
- conn.host,
- options=extra_options,
- basic_auth=(conn.login, conn.password),
- get_server_info=get_server_info,
- validate=validate,
+ self.client = Jira(
+ url=conn.host,
+ username=conn.login,
+ password=conn.password,
+ verify_ssl=verify,
proxies=self.proxies,
)
- except JIRAError as jira_error:
+ except Exception as jira_error:
raise AirflowException(f"Failed to create jira client, jira error: {str(jira_error)}")
Review Comment:
I don't think we need to catch the exceptions at all?
It will just raise whatever exception coming from the sdk
--
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 #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
eladkal merged PR #27633:
URL: https://github.com/apache/airflow/pull/27633
--
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] Adityamalik123 commented on a diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1036406657
##########
airflow/providers/atlassian/jira/hooks/jira.py:
##########
@@ -43,48 +43,51 @@ def __init__(self, jira_conn_id: str = default_conn_name, proxies: Any | None =
super().__init__()
self.jira_conn_id = jira_conn_id
self.proxies = proxies
- self.client: JIRA | None = None
+ self.client: Jira | None = None
self.get_conn()
- def get_conn(self) -> JIRA:
+ def get_conn(self) -> Jira:
if not self.client:
self.log.debug("Creating Jira client for conn_id: %s", self.jira_conn_id)
- get_server_info = True
- validate = True
- extra_options = {}
+ verify = True
if not self.jira_conn_id:
raise AirflowException("Failed to create jira client. no jira_conn_id provided")
conn = self.get_connection(self.jira_conn_id)
if conn.extra is not None:
extra_options = conn.extra_dejson
# only required attributes are taken for now,
- # more can be added ex: async, logging, max_retries
+ # more can be added ex: timeout, cloud, session
# verify
if "verify" in extra_options and extra_options["verify"].lower() == "false":
- extra_options["verify"] = False
+ verify = False
# validate
- if "validate" in extra_options and extra_options["validate"].lower() == "false":
- validate = False
-
- if "get_server_info" in extra_options and extra_options["get_server_info"].lower() == "false":
- get_server_info = False
+ if "validate" in extra_options:
+ warnings.warn(
+ "Passing 'validate' in the connection is no longer supported.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+
+ if "get_server_info" in extra_options:
+ warnings.warn(
+ "Passing 'get_server_info' in the connection is no longer supported.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
Review Comment:
Makes sense. I have removed these warnings.
--
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 #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #27633:
URL: https://github.com/apache/airflow/pull/27633#issuecomment-1338371738
Can you also please update `provider.yaml` and add 2.0.0 version 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
[GitHub] [airflow] Adityamalik123 commented on a diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1036413625
##########
airflow/providers/atlassian/jira/sensors/jira.py:
##########
@@ -96,38 +93,38 @@ def __init__(
if field_checker_func is None:
field_checker_func = self.issue_field_checker
- super().__init__(jira_conn_id=jira_conn_id, result_processor=field_checker_func, **kwargs)
+ super().__init__(
+ jira_conn_id=jira_conn_id, method_name="issue", result_processor=field_checker_func, **kwargs
+ )
def poke(self, context: Context) -> Any:
self.log.info("Jira Sensor checking for change in ticket: %s", self.ticket_id)
self.method_name = "issue"
- self.method_params = {"id": self.ticket_id, "fields": self.field}
+ self.method_params = {"key": self.ticket_id, "fields": self.field}
return JiraSensor.poke(self, context=context)
- def issue_field_checker(self, issue: Issue) -> bool | None:
+ def issue_field_checker(self, jira_result: dict) -> bool | None:
"""Check issue using different conditions to prepare to evaluate sensor."""
result = None
try:
- if issue is not None and self.field is not None and self.expected_value is not None:
+ if jira_result is not None and self.field is not None and self.expected_value is not None:
- field_val = getattr(issue.fields, self.field)
+ field_val = jira_result.get("fields", {}).get(self.field, None)
if field_val is not None:
if isinstance(field_val, list):
result = self.expected_value in field_val
elif isinstance(field_val, str):
result = self.expected_value.lower() == field_val.lower()
- elif isinstance(field_val, Resource) and getattr(field_val, "name"):
- result = self.expected_value.lower() == field_val.name.lower()
+ elif isinstance(field_val, dict) and field_val.get("name", None):
+ result = self.expected_value.lower() == field_val.get("name", "").lower()
else:
self.log.warning(
"Not implemented checker for issue field %s which "
"is neither string nor list nor Jira Resource",
self.field,
)
- except JIRAError as jira_error:
- self.log.error("Jira error while checking with expected value: %s", jira_error)
except Exception:
self.log.exception("Error while checking with expected value %s:", self.expected_value)
Review Comment:
Made the changes here by only keeping the HTTPError to log the response from it. Other will just raise with whatever exception that comes.
Ref: https://github.com/apache/airflow/pull/27633/files#diff-8edbc4ba86e754ffa8636bf167fa0a0e1eb9bd47f134a1d78103b3b7dad55b10R130
--
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] Adityamalik123 commented on a diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1036416427
##########
airflow/providers/atlassian/jira/hooks/jira.py:
##########
@@ -43,48 +43,51 @@ def __init__(self, jira_conn_id: str = default_conn_name, proxies: Any | None =
super().__init__()
self.jira_conn_id = jira_conn_id
self.proxies = proxies
- self.client: JIRA | None = None
+ self.client: Jira | None = None
self.get_conn()
- def get_conn(self) -> JIRA:
+ def get_conn(self) -> Jira:
if not self.client:
self.log.debug("Creating Jira client for conn_id: %s", self.jira_conn_id)
- get_server_info = True
- validate = True
- extra_options = {}
+ verify = True
if not self.jira_conn_id:
raise AirflowException("Failed to create jira client. no jira_conn_id provided")
conn = self.get_connection(self.jira_conn_id)
if conn.extra is not None:
extra_options = conn.extra_dejson
# only required attributes are taken for now,
- # more can be added ex: async, logging, max_retries
+ # more can be added ex: timeout, cloud, session
# verify
if "verify" in extra_options and extra_options["verify"].lower() == "false":
- extra_options["verify"] = False
+ verify = False
# validate
- if "validate" in extra_options and extra_options["validate"].lower() == "false":
- validate = False
-
- if "get_server_info" in extra_options and extra_options["get_server_info"].lower() == "false":
- get_server_info = False
+ if "validate" in extra_options:
+ warnings.warn(
+ "Passing 'validate' in the connection is no longer supported.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+
+ if "get_server_info" in extra_options:
+ warnings.warn(
+ "Passing 'get_server_info' in the connection is no longer supported.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
try:
- self.client = JIRA(
- conn.host,
- options=extra_options,
- basic_auth=(conn.login, conn.password),
- get_server_info=get_server_info,
- validate=validate,
+ self.client = Jira(
+ url=conn.host,
+ username=conn.login,
+ password=conn.password,
+ verify_ssl=verify,
proxies=self.proxies,
)
- except JIRAError as jira_error:
+ except Exception as jira_error:
raise AirflowException(f"Failed to create jira client, jira error: {str(jira_error)}")
Review Comment:
Made the changes here by only keeping the HTTPError to log the response from it. Other will just raise with whatever exception that comes. Thanks for identifying this.
--
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] Adityamalik123 commented on a diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1036408954
##########
airflow/providers/atlassian/jira/operators/jira.py:
##########
@@ -76,16 +76,11 @@ def execute(self, context: Context) -> Any:
hook = JiraHook(jira_conn_id=self.jira_conn_id)
resource = hook.client
- # Current Jira-Python SDK (1.0.7) has issue with pickling the jira response.
- # ex: self.xcom_push(context, key='operator_response', value=jira_response)
- # This could potentially throw error if jira_result is not picklable
jira_result = getattr(resource, self.method_name)(**self.jira_method_args)
if self.result_processor:
return self.result_processor(context, jira_result)
Review Comment:
The official SDK returns 'id' in the output in all jira issue related methods. I have added these https://github.com/apache/airflow/pull/27633/files#diff-85731758409b94fa7ff2ab57860f1d4242f7a196768f003e7b618d88254afba4R84-R85 which takes id from the response and performs `xcom_push` with that.
--
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 #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #27633:
URL: https://github.com/apache/airflow/pull/27633#issuecomment-1333123934
> > Lets also add entry in the provider change log explaining this breaking change
>
> @eladkal Can i add it in the CHANGELOG.rst under a new version or do i have to comment it here and the release manager adds it?
Yeah. Check opsgenie change log for example as it had similar case of changing sdk
--
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] Adityamalik123 commented on pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on PR #27633:
URL: https://github.com/apache/airflow/pull/27633#issuecomment-1339260543
> Can you also please update `provider.yaml` and add 2.0.0 version there?
Sure, I have pushed the same.
--
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] Adityamalik123 commented on a diff in pull request #27633: Changing atlassian JIRA SDK to official atlassian-python-api SDK
Posted by GitBox <gi...@apache.org>.
Adityamalik123 commented on code in PR #27633:
URL: https://github.com/apache/airflow/pull/27633#discussion_r1039375129
##########
airflow/providers/atlassian/jira/operators/jira.py:
##########
@@ -76,16 +78,14 @@ def execute(self, context: Context) -> Any:
hook = JiraHook(jira_conn_id=self.jira_conn_id)
resource = hook.client
- # Current Jira-Python SDK (1.0.7) has issue with pickling the jira response.
- # ex: self.xcom_push(context, key='operator_response', value=jira_response)
- # This could potentially throw error if jira_result is not picklable
jira_result = getattr(resource, self.method_name)(**self.jira_method_args)
+
+ output = jira_result.get("id", None) if jira_result is not None else None
+ self.xcom_push(context, key="id", value=output)
+
if self.result_processor:
return self.result_processor(context, jira_result)
return jira_result
-
- except JIRAError as jira_error:
- raise AirflowException(f"Failed to execute jiraOperator, error: {str(jira_error)}")
- except Exception as e:
- raise AirflowException(f"Jira operator error: {str(e)}")
+ except HTTPError as e:
+ raise AirflowException(f"Failed to execute jiraOperator, error: {str(e.response)}")
Review Comment:
> I think we don't really need this? the error from the sdk will simply populate to Airflow and will fail the task.. there is not need to wrap it with Airflow exception
I just thought since we (provider code) are calling the external library, we should log the returned response in case of failure. Without this Its only gonna log the traceback by default.
But yeah its not needed necessarily and i can remove it if you'd like.
--
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