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 2023/01/09 01:16:17 UTC
[GitHub] [airflow] bugraoz93 opened a new pull request, #28795: Migrate Models Variable to Internal API
bugraoz93 opened a new pull request, #28795:
URL: https://github.com/apache/airflow/pull/28795
related: #28271
Migration of `Models.Variable` to Internal API.
- set
- update
- delete
--
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 a diff in pull request #28795: Migrate Models Variable to Internal API
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28795:
URL: https://github.com/apache/airflow/pull/28795#discussion_r1081924148
##########
airflow/models/variable.py:
##########
@@ -181,6 +183,7 @@ def set(
@classmethod
Review Comment:
Why not @staticmethod here (As opposed to set above)?
--
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 #28795: Migrate Models Variable to Internal API
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #28795:
URL: https://github.com/apache/airflow/pull/28795#issuecomment-1399154745
Needs conflict resolution after other merges.
--
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 a diff in pull request #28795: Migrate Models Variable to Internal API
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28795:
URL: https://github.com/apache/airflow/pull/28795#discussion_r1081960057
##########
airflow/models/variable.py:
##########
@@ -181,6 +183,7 @@ def set(
@classmethod
Review Comment:
Then also `cls` has to be 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] bugraoz93 commented on pull request #28795: Migrate Models Variable to Internal API
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #28795:
URL: https://github.com/apache/airflow/pull/28795#issuecomment-1399665080
You are right, resolved.
--
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] bugraoz93 commented on a diff in pull request #28795: Migrate Models Variable to Internal API
Posted by GitBox <gi...@apache.org>.
bugraoz93 commented on code in PR #28795:
URL: https://github.com/apache/airflow/pull/28795#discussion_r1081958464
##########
airflow/models/variable.py:
##########
@@ -181,6 +183,7 @@ def set(
@classmethod
Review Comment:
My bad. Thanks for pointing it out! It should be **@staticmethod**. I changed it to **@staticmethod**.
--
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] bugraoz93 commented on a diff in pull request #28795: Migrate Models Variable to Internal API
Posted by GitBox <gi...@apache.org>.
bugraoz93 commented on code in PR #28795:
URL: https://github.com/apache/airflow/pull/28795#discussion_r1081962687
##########
airflow/models/variable.py:
##########
@@ -181,6 +183,7 @@ def set(
@classmethod
Review Comment:
Definitely :)
--
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 #28795: Migrate Models Variable to Internal API
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #28795:
URL: https://github.com/apache/airflow/pull/28795#issuecomment-1399580021
And yeah - re-rebasing (confllicts resolution) is needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #28795: Migrate Models Variable to Internal API
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #28795:
URL: https://github.com/apache/airflow/pull/28795
--
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