[GitHub] [airflow] mik-laj commented on pull request #9170: [WIP] Read only endpoint for XCom #8134

2020-06-10 Thread GitBox 


mik-laj commented on pull request #9170:
URL: https://github.com/apache/airflow/pull/9170#issuecomment-641929106


   @ashb Personally, I prefer explicite joins.  Most of our contributors are 
SQL experts, so the current approach for them is more natural.  We can start 
using them, but I think it's worth trying to migrate some of the existing code 
to find out the real limitations and problems. We may not have the shortest 
code now, but it is readable by our contributors and trusted.
   
   We already had a similar situation in the project. We wanted to start using 
assertions, fixtures and other new solutions for writing the test. We had 
discussions and votes on whether this is a good idea. Few people who tried to 
use it in practice. I wanted to use it for writing the API and gave up. 
   There were two main problems. assert statements  The assertion expression 
displays much less readable error messages. Fixtures are not compatible with 
provide_session decorator.
   
   Based on this experience, I think it's best to introduce a new idea by 
trying to rewrite part of the code. Otherwise, the idea you are proposing 
affects many users, so it will be better when we discuss it on the mailing 
list. 
   



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:
us...@infra.apache.org

[GitHub] [airflow] mik-laj commented on pull request #9170: [WIP] Read only endpoint for XCom #8134

2020-06-09 Thread GitBox 


mik-laj commented on pull request #9170:
URL: https://github.com/apache/airflow/pull/9170#issuecomment-641653054


   @ashb It looks good, but I don't know if it suits our code style.  We use 
explicit joins instead of relying on ORM generated more often. Generating 
joints by ORM together with the use of multi-column primary keys can lead to 
spaghetti. Each model will have a relationship with each model because many 
values are repeated.  We only use ORM-based relationships in one place - 
DagModel.tags   It fits here because we can easily retrieve the 
object(DagModel) and metadata. (DagTag).  In other places, we need careful 
control to ensure good performance.
   
   I also prepared an example.
   
https://github.com/apache/airflow/pull/9170/commits/16f829927859604336013dd7a9755fcf7000f786
   ```python
   query = session.query(XCom)
   query = query.filter(and_(XCom.dag_id == dag_id,
 XCom.task_id == task_id,
 XCom.key == xcom_key))
   query = query.join(DR, and_(XCom.dag_id == DR.dag_id, 
XCom.execution_date == DR.execution_date))
   query = query.filter(DR.run_id == dag_run_id)
   
   q_object = query.one_or_none()
   ```
   



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:
us...@infra.apache.org