[task_panels] Add params with kibana url and version to create the .kibana #80
Conversation
mordred/task_panels.py
Outdated
| "kbn-version": self.conf['panels']['kibiter_version'] | ||
| } | ||
|
|
||
| res = requests.post(k6_init_url, headers=k6_init_headers, |
There was a problem hiding this comment.
If you extend the exception handle to cover this call to requests.post, it will catch also the case when the "name or service not found" error happens. Do you think that's worthwhile?
There was a problem hiding this comment.
Yes, good point. My fault not to include the POST call in the try. This is why it was failing to you. Changed!
| @@ -247,6 +247,16 @@ def general_params(cls): | |||
| "optional": True, | |||
| "default": "git", | |||
| "type": str | |||
| }, | |||
| "kibiter_url": { | |||
There was a problem hiding this comment.
Wouldn't it better to have a new section for Kibiter, and have these two new options in it?
There was a problem hiding this comment.
I think that probably we need to change the panel section name with kibiter name. All the params in the panelsection are related to kibiter. But this change breaks all current config files so I prefer to do it once the platform is more stable without so many changes. This is why I have added those new params to the panelsection.
There was a problem hiding this comment.
OK, now I understand. Looking at the parameters, they are more like parameters of the dashboard than parameters of the service (in the sense that to produce the same dashboard on a different deployment, those parameters wouldn't change). That´s why I still see a reason for having a separate kibiter section.
But I see your point. So, you decide. If you prefer, keep it like it is now.
There was a problem hiding this comment.
Yes, I prefer to maintain to keep as it is now and in a next iteration rename the full section to Kibiter. So I think we are ready to merge this PR? l let that you do it! Thanks @jgbarah
mordred/task_panels.py
Outdated
| ES6_KIBANA_INIT_DATA = '{"value": "*"}' | ||
| # We need the version before the .kibana exists so we can not find it | ||
| ES6_KIBANA_VERSION = "6.1.0-1" | ||
| ES6_KIBANA_INIT_HEADERS = { |
There was a problem hiding this comment.
I would rather keep this constant, to easy the code below (and spare the variable k6_init_headers, and its assignment on the fly).
There was a problem hiding this comment.
So the only global constant is then ES6_KIBANA_INIT_DATA because ES6_KIBANA_VERSION is now defined in mordred config. I have left then:
ES6_KIBANA_INIT_DATA = '{"value": "*"}'
ES6_KIBANA_INIT_URL_PATH = "/api/kibana/settings/indexPattern:placeholder"
There was a problem hiding this comment.
I was referring mainly to ES_KIBANA_INIT_HEADERS, sorry for not being clear enough.
There was a problem hiding this comment.
You can not define this as a global constant because it depends of the version that is a variable defined in mordred config (self.conf['panels']['kibiter_version']), and it is not available when this global variable will be defined.
The headers could be defined partially globally:
ES_KIBITER_INIT_HEADERS = {
"Accept": "application/json",
"Content-Type": "application/json",
"kbn-version": None
}
and later, complete it with the kbn-version value but it does not make sense IMHO (I found it too complex).
There was a problem hiding this comment.
I was meaning something like you say, but I agree it is a bit weird. So, if you prefer, let's keep it as it is now.
|
@jgb all your comments addressed. Could you review them? Thanks! |
|
OK. I'll open a ticket about the section name to propose opening a But now, it seems there are conflicts, maybe due to your latest commits. Would you mind rebasing? |
…index Use global constants to define the Kibiter URL to create the .kibana index. Manage the exception if the URL call fails.
acs
force-pushed
the
kibana-index-init-fix
branch
from
2bdaf2b
to
f425b3e
Compare
|
Rebase completed. Could you then merge the PR? Thanks! |
No description provided.