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 2021/01/01 03:08:57 UTC

[GitHub] [airflow] kubatyszko opened a new pull request #13356: Kuba openfaas sync call

kubatyszko opened a new pull request #13356:
URL: https://github.com/apache/airflow/pull/13356


   Adding a way to invoke openfaas functions synchronously, waiting for the invocation to complete.
   
   ---
   
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kubatyszko commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
kubatyszko commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-753273934


   Ok, I think I got it as far as I can, now it's failing with rabbitMQ error, assuming unrelated to my code:
   
   `===============================================================================================
   609
                  Checking integrations and backends
   610
     ===============================================================================================
   611
     -----------------------------------------------------------------------------------------------
   612
     Kerberos: .OK.  
   613
     MongoDB: OK.  
   614
     Redis: OK.  
   615
     RabbitMQ: ....................ERROR: Maximum number of retries while checking service. Exiting 
   616
     Service could not be started!
   617
     
   618
     $ nc -zvv rabbitmq 5672
   619
     DNS fwd/rev mismatch: rabbitmq != docker-compose_rabbitmq_1.docker-compose_default
   620
     rabbitmq [172.19.0.7] 5672 (amqp) : Connection refused
   621
      sent 0, rcvd 0
   622
    `


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-753282716


   It's a transient error - it happens sometimes. But as such - it can be merged I think. One last 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kubatyszko closed pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
kubatyszko closed pull request #13356:
URL: https://github.com/apache/airflow/pull/13356


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kubatyszko closed pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
kubatyszko closed pull request #13356:
URL: https://github.com/apache/airflow/pull/13356


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kubatyszko commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
kubatyszko commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-753249642


   @potiuk static checks succeeded finally, some other (seemingly unrelated) checks are failing.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk merged pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #13356:
URL: https://github.com/apache/airflow/pull/13356


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-753284678


   Nothing to look for . Not much different than 2020 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-752726100


   [The Workflow run](https://github.com/apache/airflow/actions/runs/453187391) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kubatyszko closed pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
kubatyszko closed pull request #13356:
URL: https://github.com/apache/airflow/pull/13356


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-752949350


   We've fixed failing master (the failing tests were due to that). Fixing the static checks and rebasing shoudl make it green :)


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-753240964


   [The Workflow run](https://github.com/apache/airflow/actions/runs/455422218) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kubatyszko commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
kubatyszko commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-753283229


   Woohoo, thank you ! (and as of 2 minutes ago I'm finally in 2021 ;) )


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kubatyszko commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
kubatyszko commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-753253543


   Hmm, something wrong with mock_requests. It doesn't even work for the async openfaas call (which I did not modify):
   
   `
   3170
     >       raise exceptions.NoMockAddress(request)
   3171
     E       requests_mock.exceptions.NoMockAddress: No mock address: POST http://open-faas.io/async-function/function_name
   3172
     
   3173
     /usr/local/lib/python3.6/site-packages/requests_mock/adapter.py:258: NoMockAddress`


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-752712860


   Also there are some static code failures :). I recommend using `pre-commit`


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#discussion_r550283152



##########
File path: airflow/providers/openfaas/hooks/openfaas.py
##########
@@ -68,16 +69,32 @@ def deploy_function(self, overwrite_function_if_exist: bool, body: Dict[str, Any
                 self.log.info("Function deployed %s", self.function_name)
 
     def invoke_async_function(self, body: Dict[str, Any]) -> None:
-        """Invoking function"""
+        """Invoking function asynchronously"""
         url = self.get_conn().host + self.INVOKE_ASYNC_FUNCTION + self.function_name
-        self.log.info("Invoking function %s", url)
+        self.log.info("Invoking function asynchronously %s", url)
         response = requests.post(url, body)

Review comment:
       Should we use `json=body` consistently ? Right now it looks like they might encode the data differently (`json=body` will be json-encoded and `body` will be form-encoded). Was this  deliberate distinction? https://docs.openfaas.com/reference/async/ does not seem to indicate there is any difference.  




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk edited a comment on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-752949350


   We've fixed failing master (the failing tests were due to that). Fixing the static checks and rebasing should make it green :)


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#issuecomment-753284618


   :tada: :tada: :tada: :tada: :tada: :tada: :tada: 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kubatyszko commented on a change in pull request #13356: Kuba openfaas sync call

Posted by GitBox <gi...@apache.org>.
kubatyszko commented on a change in pull request #13356:
URL: https://github.com/apache/airflow/pull/13356#discussion_r550290863



##########
File path: airflow/providers/openfaas/hooks/openfaas.py
##########
@@ -68,16 +69,32 @@ def deploy_function(self, overwrite_function_if_exist: bool, body: Dict[str, Any
                 self.log.info("Function deployed %s", self.function_name)
 
     def invoke_async_function(self, body: Dict[str, Any]) -> None:
-        """Invoking function"""
+        """Invoking function asynchronously"""
         url = self.get_conn().host + self.INVOKE_ASYNC_FUNCTION + self.function_name
-        self.log.info("Invoking function %s", url)
+        self.log.info("Invoking function asynchronously %s", url)
         response = requests.post(url, body)

Review comment:
       I've corrected 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org