pierrejeambrun commented on code in PR #27805:
URL: https://github.com/apache/airflow/pull/27805#discussion_r1033021606
##########
airflow/www/templates/airflow/trigger.html:
##########
@@ -44,12 +44,23 @@ <h2>Trigger DAG: {{ dag_id }}</h2>
<div class="form-group row">
<div class="col-md-2">
<label for="run_id">Run id (Optional)</label>
- <input type="text" class="form-control" placeholder="Run ID"
name="run_id" >
+ <input type="text" class="form-control" placeholder="Run ID"
name="run_id">
+ </div>
+ </div>
+ <div class="form-group row">
+ <div class="col-md-2">
+ <label for="recent_configs">Recent Configurations</label>
+ <select class="form-control" name="recent_configs" id="recent_configs">
+ <option value="">None</option>
+ {% for conf in recent_confs %}
+ <option value="{{conf}}">{{ conf }}</option>
+ {% endfor %}
+ </select>
</div>
</div>
<label for="conf">Configuration JSON (Optional, must be a dict
object)</label>
<textarea class="form-control" name="conf" id="json">{{ conf }}</textarea>
- </div>
+
Review Comment:
This div was here to adding some spacing after the json editor. (It wasn't
working properly indeed). Maybe adding a line break (<br>), currently it's
quite compact :):
##########
airflow/www/templates/airflow/trigger.html:
##########
@@ -44,12 +44,23 @@ <h2>Trigger DAG: {{ dag_id }}</h2>
<div class="form-group row">
<div class="col-md-2">
<label for="run_id">Run id (Optional)</label>
- <input type="text" class="form-control" placeholder="Run ID"
name="run_id" >
+ <input type="text" class="form-control" placeholder="Run ID"
name="run_id">
+ </div>
+ </div>
+ <div class="form-group row">
+ <div class="col-md-2">
+ <label for="recent_configs">Recent Configurations</label>
+ <select class="form-control" name="recent_configs" id="recent_configs">
+ <option value="">None</option>
Review Comment:
Maybe we can display something like 'empty config' instead of None that
doesn't provide much information to the user ?
##########
airflow/www/views.py:
##########
@@ -1855,6 +1855,19 @@ def trigger(self, session=None):
flash(f"Cannot create dagruns because the dag {dag_id} has import
errors", "error")
return redirect(origin)
+ recent_confs = (
+ session.query(DagRun.conf)
+ .filter(
+ DagRun.dag_id == dag_id,
+ DagRun.run_type == DagRunType.MANUAL,
+ DagRun.conf is not None,
Review Comment:
I don't think empty objects are relevent here. (also update the `is not
None` syntax)
```suggestion
DagRun.conf.isnot(None),
DagRun.conf != {}
```
##########
airflow/www/static/js/trigger.js:
##########
@@ -20,13 +20,24 @@
/* global document, CodeMirror, window */
const textArea = document.getElementById('json');
+const recentConfigList = document.getElementById('recent_configs');
const minHeight = 300;
const maxHeight = window.innerHeight - 450;
const height = maxHeight > minHeight ? maxHeight : minHeight;
CodeMirror.fromTextArea(textArea, {
lineNumbers: true,
- mode: { name: 'javascript', json: true },
+ mode: {
+ name: 'javascript',
+ json: true,
+ },
gutters: ['CodeMirror-lint-markers'],
lint: true,
-}).setSize(null, height);
+})
+ .setSize(null, height);
+
+function setRecentConfig(e) {
+
document.querySelector('.CodeMirror').CodeMirror.setValue(e.target.value.replaceAll("'",
'"'));
Review Comment:
To avoid manually replacing the quotes, you can `json.dumps(conf)`. See what
is done in the grid_data endpoint, and my other comments.
##########
airflow/www/views.py:
##########
@@ -1855,6 +1855,19 @@ def trigger(self, session=None):
flash(f"Cannot create dagruns because the dag {dag_id} has import
errors", "error")
return redirect(origin)
+ recent_confs = (
+ session.query(DagRun.conf)
+ .filter(
+ DagRun.dag_id == dag_id,
+ DagRun.run_type == DagRunType.MANUAL,
+ DagRun.conf is not None,
+ )
+ .order_by(DagRun.execution_date.desc())
+ .limit(5)
+ .all()
+ )
+ recent_confs = [getattr(conf, "conf") for conf in recent_confs]
Review Comment:
Conf are not always json compatible. I believe they can be plain 'str'.
check `def encode_dag_run` func. We may want to also handle this case. Maybe
extract a reusable function that will handle the conf. (`json.dumps` if
possible, otherwise str, and return `conf_is_json`, and skip the conf that are
not json in the response)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
