[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026]
Add filter by DAG tags
URL: https://github.com/apache/airflow/pull/6489#discussion_r347502474
##
File path: airflow/www/views.py
##
@@ -213,6 +218,18 @@ def get_int_arg(value, default=0):
arg_current_page = request.args.get('page', '0')
arg_search_query = request.args.get('search', None)
+arg_tags_filter = request.args.getlist('tags', None)
+flask_session.permanent = True
Review comment:
What does this do? Convert the it from a session cookie to a permanent one?
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
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags URL: https://github.com/apache/airflow/pull/6489#discussion_r347492662 ## File path: airflow/www/utils.py ## @@ -471,9 +471,12 @@ def clean_column_names(): def is_utcdatetime(self, col_name): from airflow.utils.sqlalchemy import UtcDateTime -obj = self.list_columns[col_name].type -return isinstance(obj, UtcDateTime) or \ -isinstance(obj, sqla.types.TypeDecorator) and \ -isinstance(obj.impl, UtcDateTime) + +if col_name in self.list_columns: Review comment: What was this change for btw? 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 With regards, Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026]
Add filter by DAG tags
URL: https://github.com/apache/airflow/pull/6489#discussion_r347492287
##
File path: airflow/www/templates/airflow/dags.html
##
@@ -81,9 +92,17 @@ DAGs
-
-{{ dag.dag_id }}
-
+
Review comment:
Do we need to have both float - isn't just floating the tags to the right
enough?
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
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026]
Add filter by DAG tags
URL: https://github.com/apache/airflow/pull/6489#discussion_r347491899
##
File path: airflow/www/app.py
##
@@ -65,6 +66,7 @@ def create_app(config=None, session=None, testing=False,
app_name="Airflow"):
app.config['SESSION_COOKIE_HTTPONLY'] = True
app.config['SESSION_COOKIE_SECURE'] = conf.getboolean('webserver',
'COOKIE_SECURE')
app.config['SESSION_COOKIE_SAMESITE'] = conf.get('webserver',
'COOKIE_SAMESITE')
+app.config['PERMANENT_SESSION_LIFETIME'] = timedelta(days=3560) # 10 years
Review comment:
This should either come from a config, or be set before line 61 (where we do
`app.config.from_pyfile`) so that this setting can be changed from
webserver_config.py.
I think a 10year default is a bit too long a default too :) 14 or 30 days
seems more reasonable
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
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags URL: https://github.com/apache/airflow/pull/6489#discussion_r347464559 ## File path: airflow/models/dag.py ## @@ -1384,7 +1390,8 @@ def sync_to_db(self, owner=None, sync_time=None, session=None): orm_dag.default_view = self._default_view orm_dag.description = self.description orm_dag.schedule_interval = self.schedule_interval -session.merge(orm_dag) +orm_dag.tags = self.get_dagtags(session=session) Review comment: Oh. It's possible, but we have to handle the DB differences our selves :( https://stackoverflow.com/questions/39470239/sqlalchemy-json-column-how-to-preform-a-contains-query 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 With regards, Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags URL: https://github.com/apache/airflow/pull/6489#discussion_r347088953 ## File path: airflow/kubernetes/pod_generator.py ## @@ -26,7 +26,7 @@ import kubernetes.client.models as k8s -from airflow.executors import Executors +from airflow.executors import Executors # pylint: disable=cyclic-import Review comment: Given you otherwise didn't change these files I'm surprised you need 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026]
Add filter by DAG tags
URL: https://github.com/apache/airflow/pull/6489#discussion_r347021605
##
File path: airflow/www/views.py
##
@@ -156,12 +158,19 @@ class AirflowBaseView(BaseView):
}
def render_template(self, *args, **kwargs):
-return super().render_template(
+templ = super().render_template(
*args,
# Cache this at most once per request, not for the lifetime of the
view instance
scheduler_job=lazy_object_proxy.Proxy(jobs.SchedulerJob.most_recent_job),
**kwargs
)
+resp = make_response(templ)
+for k, v in kwargs.get('cookies', {}).items():
+if v:
+resp.set_cookie(k, v, expires=2147483647)
Review comment:
What is this doing? What is the magic number here?
Can we instead make use of the existing Flask session and store something in
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags URL: https://github.com/apache/airflow/pull/6489#discussion_r347020711 ## File path: airflow/models/dag.py ## @@ -1384,7 +1390,8 @@ def sync_to_db(self, owner=None, sync_time=None, session=None): orm_dag.default_view = self._default_view orm_dag.description = self.description orm_dag.schedule_interval = self.schedule_interval -session.merge(orm_dag) +orm_dag.tags = self.get_dagtags(session=session) Review comment: I wonder if it would be better to use a JSON column on the Dag table, rather than needing a new dag-tag table. SQLAlchemy has built in support for JSON columns for mysql, sqlite and postgres, and with postgres we could even add an index on 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags URL: https://github.com/apache/airflow/pull/6489#discussion_r347019417 ## File path: airflow/kubernetes/pod_generator.py ## @@ -26,7 +26,7 @@ import kubernetes.client.models as k8s -from airflow.executors import Executors +from airflow.executors import Executors # pylint: disable=cyclic-import Review comment: ? 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 With regards, Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags
ashb commented on a change in pull request #6489: [AIRFLOW-3959] [AIRFLOW-4026] Add filter by DAG tags URL: https://github.com/apache/airflow/pull/6489#discussion_r347019518 ## File path: airflow/kubernetes/pod_launcher.py ## @@ -27,7 +27,7 @@ from kubernetes.stream import stream as kubernetes_stream from requests.exceptions import BaseHTTPError -from airflow import AirflowException +from airflow import AirflowException # pylint: disable=cyclic-import Review comment: Here too? 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 With regards, Apache Git Services
