dmsimard | apollo13: doing code reviews can be helpful too btw, the changes are pretty focused and simple (I think) | 00:38 |
---|---|---|
dmsimard | You can set yourself a dashboard based on searches, for example: https://review.openstack.org/#/q/project:%255Eopenstack/ara-.*+status:open | 00:38 |
*** njt has quit IRC | 07:12 | |
*** njt has joined #ara | 07:12 | |
*** gvincent has joined #ara | 07:53 | |
*** jungleslow has quit IRC | 08:09 | |
*** jungleslow has joined #ara | 11:11 | |
jungleslow | Hey guys ! Ara is really great ! Congrats for all your efforts | 11:13 |
jungleslow | Can we do authentication with the current version ? | 11:15 |
dmsimard | jungleslow: no, the web interface is read only and authentication can be done with a web server like nginx or apache | 11:38 |
dmsimard | There won't be any authentication/permissions/acl/rbac even in the upcoming 1.0 release. This is so far by design to keep things simple. | 11:39 |
jungleslow | Sure, thanks a lot | 11:40 |
*** themurph has joined #ara | 11:41 | |
jungleslow | Another question, I'm using apache and postgresql. So I'm connecting to the DB using in my ansible.cfg "database = postgresql+psycopg2://..." It's working with the password but now I want to hide it within git or with vault, is it possible ? | 11:50 |
dmsimard | jungleslow: in 1.0 there'll be an API which will make this a bit more straightforward but for now it needs to be there | 12:06 |
dmsimard | How you put the data in your ansible.cfg is up to you, though | 12:06 |
dmsimard | I mean, you can have a role that retrieves your database password from vault and put it there | 12:07 |
jungleslow | Oh I see. When do you plan to release Ara 1.0 ? :D | 12:11 |
dmsimard | I've been working on it for >1 year now ? So any time soon I guess ? :p | 12:11 |
jungleslow | Nice ! Good luck dude ! | 12:12 |
dmsimard | If you want to look: https://github.com/openstack/ara-server | 12:12 |
dmsimard | https://github.com/openstack/ara-server/blob/master/doc/source/_static/graphs/recording-workflow.png | 12:14 |
dmsimard | Er, I meant to link https://github.com/openstack/ara-server/blob/master/doc/source/arch.rst | 12:14 |
jungleslow | Thanks I will have a look ! | 12:22 |
*** themurph has quit IRC | 12:46 | |
*** themurph has joined #ara | 12:59 | |
*** jrist has quit IRC | 13:34 | |
*** jrist has joined #ara | 13:38 | |
*** themurph has quit IRC | 13:44 | |
*** themurph has joined #ara | 13:46 | |
*** sshnaidm_ has joined #ara | 13:53 | |
*** sshnaidm has quit IRC | 13:54 | |
gnupyx | dmsimard, hi, the graph is for the 1.0 release? | 14:00 |
dmsimard | gnupyx: that particular graph explains a little bit how 1.0 works, yes | 14:03 |
dmsimard | gnupyx: 0.x is similar but doesn't have an API | 14:03 |
dmsimard | and everything is coupled together into the same backend | 14:03 |
*** tbielawa has joined #ara | 14:04 | |
gnupyx | yes I saw today that there is no API in ara 0.16 :) | 14:24 |
*** sshnaidm_ has quit IRC | 14:25 | |
dmsimard | 1.0 is almost ready for wider beta testing. It's no longer a matter of months, it's a matter of weeks | 14:33 |
*** sshnaidm_ has joined #ara | 14:45 | |
apollo13 | dmsimard: around? | 14:50 |
dmsimard | apollo13: asquare | 14:51 |
apollo13 | what does that mean :) | 14:52 |
dmsimard | a round | 14:52 |
dmsimard | a square | 14:52 |
dmsimard | different shapes | 14:52 |
apollo13 | oh | 14:52 |
dmsimard | sorry I'm tired lol | 14:52 |
apollo13 | not my day for that :D | 14:52 |
dmsimard | apollo13: how cna I help | 14:53 |
apollo13 | dmsimard: do we need the nested router for playbook/files? | 14:54 |
apollo13 | I mean it is all nice and well, but what does it offer over the normal files endpoint? | 14:54 |
dmsimard | apollo13: I need to go through my logs with gvincent to remember why it was relevant | 14:54 |
dmsimard | give me a few | 14:54 |
apollo13 | mhm, maybe because it was a m2m; I am just wondering if we want to "enfore" the extra package just so we can have a little bit nicer url | 14:56 |
*** tbielawa is now known as tbielawa|mtg | 14:59 | |
dmsimard | apollo13: so... between reading old discussions and looking at today's code | 14:59 |
dmsimard | I'm failing to understand why we moved away from tying everything back to a playbook | 14:59 |
dmsimard | in 0.x, almost everything has a fk to playbook.id | 14:59 |
dmsimard | (including files) | 14:59 |
dmsimard | OH | 15:00 |
dmsimard | I remember | 15:00 |
dmsimard | sort of | 15:00 |
* dmsimard hamster running | 15:00 | |
apollo13 | yeah that would have been the next question; before I attack filtering and so I would have tied results and tasks to playbooks if you still think it should be there | 15:01 |
dmsimard | So in 0.x, we have this very very ugly piece of code: https://github.com/openstack/ara/blob/master/ara/models.py#L199-L239 | 15:01 |
dmsimard | The gist of the issue being that there was a very racey condition where the playbook could have been created but not it's file (yet) | 15:02 |
dmsimard | but yeah, in hindsight | 15:03 |
dmsimard | 1) I'm not convinced we need a nested router | 15:04 |
apollo13 | how did you manage that in 0.x? I mean if you use transactions and all it would be all or nothing | 15:04 |
dmsimard | 2) I think we probably need to add the playbook.id fk to the models that are missing it | 15:04 |
dmsimard | apollo13: someone hitting ctrl+c at the wrong time | 15:04 |
apollo13 | even then, a (db) transaction is all or nothing | 15:04 |
dmsimard | the file addition came later | 15:05 |
dmsimard | because to add the file, you need the playbook id :) | 15:05 |
dmsimard | because you need to tie the file to the playbook | 15:05 |
apollo13 | sure, but one db transaction… | 15:05 |
dmsimard | ¯\_(ツ)_/¯ | 15:05 |
apollo13 | okay | 15:05 |
dmsimard | anyway | 15:05 |
apollo13 | so it is not because ansible didn't give you access to the file yet or though | 15:05 |
dmsimard | I think we can handle this better in 1.0 | 15:05 |
dmsimard | right, it was someone hitting ctrl+c at a very unfortunate moment | 15:06 |
apollo13 | speaking from an API perspective, it would be great if one could for instance create a playbook with it's acompaning file via one API call | 15:06 |
dmsimard | at least that's my understanding of it | 15:06 |
apollo13 | otherwise you have to prepare for all situations that can happen in between | 15:06 |
dmsimard | apollo13: I think that was more or less the entire point of that nested router | 15:06 |
dmsimard | (creating playbook and file together) | 15:07 |
apollo13 | ok, so lets reevaluate that as we go and I'll add the playbook as a start to everywhere | 15:07 |
apollo13 | ok, cause the nested router currently fails greatly at doing that as far as I can see :D | 15:07 |
dmsimard | I'm not sure if we're even using it | 15:07 |
dmsimard | I think the callback uses files directly | 15:08 |
apollo13 | perfect, I'll also remove it in a review request then and see what the integration tests say | 15:08 |
dmsimard | apollo13: oh, it does use the nested router https://github.com/openstack/ara-plugins/blob/master/ara/plugins/callback/ara_default.py#L231-L238 | 15:08 |
dmsimard | however, at that point, we already have a playbook id available so we can fairly easily substitute that to use /api/v1/files instead | 15:09 |
apollo13 | ok, but nothing in there forces it to be nested | 15:09 |
apollo13 | yes | 15:09 |
dmsimard | so we need to add playbook.id to files | 15:09 |
dmsimard | playbook.id to tasks and playbook.id to results | 15:09 |
dmsimard | afaict | 15:09 |
dmsimard | let me merge the batch of changes in flight since it impacts the model | 15:10 |
apollo13 | well which would bring up one question about the datastructure as a whole | 15:10 |
apollo13 | if File points to playbook; the m2m from playbook to files beconmes redundant | 15:10 |
apollo13 | it also means that a file can only belong to a single playbook, which was probably the intention in the first place | 15:11 |
apollo13 | (file contents are shared but files are unique) | 15:11 |
dmsimard | apollo13: yes | 15:11 |
apollo13 | on the other hand, there is also a foreignkey from playbook to file | 15:12 |
apollo13 | that one will become rather weird :D | 15:12 |
apollo13 | that file link is probably for the root play file itself? | 15:12 |
dmsimard | yeah, we can remove that fk | 15:12 |
dmsimard | the objective here | 15:12 |
dmsimard | is to positively identify the playbook file | 15:12 |
dmsimard | vs any other file (tasks, roles, etc.) | 15:12 |
dmsimard | in 0.x, there's a field in files "is_playbook" | 15:13 |
*** gvincent has quit IRC | 15:13 | |
apollo13 | okay, what about a filetype choice field instead? | 15:13 |
apollo13 | like it would be interesting to know if a file is a template or a task etc… | 15:13 |
apollo13 | we do not have to use it initially, just as marker for is_playbook, but in the longrun it would be nice to mark all types | 15:14 |
dmsimard | sure, but I don't know if ansible provides a way to meaningfully differentiate different file types -- all it does it to provide a list | 15:14 |
dmsimard | so basically in 0.x | 15:16 |
dmsimard | we did like | 15:16 |
dmsimard | select * from files where playbook_id=foo and is_playbook=true to identify the playbook file | 15:16 |
dmsimard | which feels overkill when there could be a quick lookup somehow | 15:17 |
apollo13 | *shrug* is lookup feels rather fast if properly indexed | 15:18 |
apollo13 | this lookup* | 15:18 |
dmsimard | I don't have a strong opinion | 15:18 |
dmsimard | either way | 15:18 |
apollo13 | that is, performance wise I wouldn't rule it out | 15:18 |
apollo13 | same goes for playbook_id=foo and file_type="playbook" | 15:19 |
apollo13 | let me check the ara 0.x callback to see where and how you get the files | 15:19 |
dmsimard | yeah | 15:19 |
dmsimard | I can give you that | 15:19 |
dmsimard | apollo13: https://github.com/openstack/ara/blob/master/ara/plugins/callbacks/log_ara.py#L318-L343 | 15:19 |
apollo13 | as for types, I think we could infer it from the name | 15:19 |
*** gvincent has joined #ara | 15:19 | |
apollo13 | dmsimard: ah now I understand your race condition | 15:20 |
apollo13 | the https://github.com/openstack/ara/blob/master/ara/plugins/callbacks/log_ara.py#L340 commit right in there ;) | 15:20 |
dmsimard | yup | 15:20 |
dmsimard | but the commit is also necessary to get the ID of the playbook back iirc | 15:20 |
dmsimard | I don't know, 0.x is over 2 years old now and I don't remember everything lol | 15:21 |
apollo13 | ok and on task start you get the task file? https://github.com/openstack/ara/blob/master/ara/plugins/callbacks/log_ara.py#L284-L298 ? | 15:22 |
dmsimard | Yeah, there's a couple places where we check to make sure we have files | 15:22 |
dmsimard | Some files are included dynamically and aren't available or known to ansible when we look at first | 15:23 |
dmsimard | you can search for _load_files in 1.0 https://github.com/openstack/ara-plugins/blob/master/ara/plugins/callback/ara_default.py | 15:24 |
dmsimard | 1.0 also captures way more files than 0.x FWIW | 15:24 |
dmsimard | it actually does capture vars files, meta, handlers, etc. stuff that wasn't in 0.x | 15:24 |
apollo13 | okay, but that's where it gets interesting | 15:24 |
*** gvincent has quit IRC | 15:25 | |
dmsimard | I wrote this almost a year ago now.. I should probably write a new update lol https://dmsimard.com/2017/11/22/status-update-ara-1.0/ | 15:26 |
dmsimard | A lot of the things in that last update are still relevant but I hadn't yet canned the 1.0 flask branch | 15:26 |
dmsimard | I should probably tag that and delete the branch | 15:27 |
apollo13 | haha, you actually playbook_id on files there :D | 15:28 |
dmsimard | if you're curious what 1.0 looked like with a flask-restful API instead: https://github.com/openstack/ara/tree/feature/1.0 | 15:28 |
dmsimard | i.e, https://github.com/openstack/ara/tree/feature/1.0/ara/api | 15:28 |
dmsimard | a lot of work went into it, thousands of lines of codes.. >100 commits :( | 15:29 |
apollo13 | what made you drop it? | 15:29 |
dmsimard | flask-restful vs django-rest-framework | 15:29 |
dmsimard | https://github.com/flask-restful/flask-restful vs https://github.com/encode/django-rest-framework | 15:30 |
dmsimard | I was running into issues with flask-restful that were not easily fixable and the project isn't very active | 15:30 |
*** tbielawa|mtg is now known as tbielawa | 15:32 | |
dmsimard | gvincent showed me a poc of the ara API with drf in a few hours .. it took a long time to make up my mind and it was a hard decision but hopefully it's the best one for the long term | 15:32 |
apollo13 | certainly not a bad one ;) | 15:33 |
dmsimard | I felt that keeping with flask-restful was just going to be digging ourselves into a hole deeper and that the longer we stayed on it, the harder it'd be to come out | 15:33 |
apollo13 | best is always relative | 15:33 |
dmsimard | well, drf provides a lot of things out of the box that flask-restful doesn't | 15:33 |
apollo13 | anyways, gotta think about a good way on how the api will look like | 15:33 |
dmsimard | the API browser is an example | 15:33 |
apollo13 | because you obviously don't want to post file contents more often than needed | 15:33 |
apollo13 | like in the end you'd probably will have specialized api endpoints that allow the creation of lets say a playbook and that will also return information on whether the playbookfile with name xyz and sha blabla already exists | 15:34 |
dmsimard | apollo13: about file contents | 15:34 |
dmsimard | I recently added a few commits to help us with that | 15:35 |
dmsimard | https://github.com/openstack/ara-server/commit/be649f16d359cd48c1e549234d58a9ddf141beb3 | 15:35 |
dmsimard | https://github.com/openstack/ara-server/commit/7f2b3c2802e3033e07c36c54b28727bfda529275 | 15:35 |
apollo13 | yeah, saw that -- that will be absolutely needed | 15:36 |
dmsimard | The hashing should be deterministic so we can run it client side for looking it up if necessary | 15:36 |
dmsimard | We need to create a *file* but we might not need to create a *file_content* | 15:36 |
apollo13 | right, one question; currently Task has a FK to file | 15:37 |
apollo13 | to be want __every__ Task have it's own file? | 15:37 |
dmsimard | no, many tasks can relate to one file | 15:37 |
dmsimard | like, if you have 15 tasks in a file | 15:37 |
apollo13 | ok, yeah | 15:37 |
dmsimard | it'll have one file and one file content | 15:37 |
dmsimard | but 15 tasks | 15:38 |
dmsimard | so again, the idea is for us to be able to determine which file this task was in | 15:38 |
apollo13 | fwiw, how married are you to sha? | 15:38 |
dmsimard | I'm not, it's just what seemed like the most straightforward and wasn't md5 | 15:38 |
dmsimard | do you have any suggestions ? | 15:39 |
apollo13 | well, I see people moving to blake2b for that kind of stuff which is in python since 3.5 or 3.6 iirc | 15:39 |
apollo13 | mhm, 3.6 | 15:41 |
dmsimard | apollo13: it's in the stdlib ? | 15:41 |
apollo13 | jupp, hashlib.blake2b | 15:42 |
apollo13 | either way, can we agree that I can open a PR to add playbook to Task and Result? and then later to File + remove files() from Playbook | 15:46 |
dmsimard | yes | 15:47 |
dmsimard | apollo13: something we need to think about also is what kind of data we should return for /api/v1/playbooks | 15:47 |
apollo13 | no files ;) | 15:48 |
dmsimard | because it's going to end up returning basically everything | 15:48 |
dmsimard | playbook.tasks, playbook.results, playbook.files etc | 15:48 |
apollo13 | yeah, less than that :) | 15:48 |
apollo13 | I mean it's nice to have a "generic" api but the callback should still perform well :D | 15:49 |
dmsimard | We should probably return a list of IDs ? I remember working on HATEOAS slightly in the flask 1.0 https://spring.io/understanding/HATEOAS | 15:49 |
apollo13 | also the web UI if it is going to use that API | 15:49 |
dmsimard | Yeah the web UI is definitely going to be using that API | 15:50 |
dmsimard | But ideally in a way that makes it react (pun intended) quickly | 15:50 |
dmsimard | So loading data just-in-time instead of pre-loading everything | 15:50 |
apollo13 | yeah, but for that you do not really need a list of ids for instance | 15:50 |
apollo13 | also that list can be long for large projects ;) | 15:51 |
dmsimard | right | 15:51 |
dmsimard | The web UI is likely going to retrieve a playbook id and then query the different other endpoints with that playbook id | 15:51 |
apollo13 | yeah | 15:52 |
apollo13 | although if that results in a shitload of api calls you might have to rethink that anyways | 15:52 |
dmsimard | well, it depends what's the best tradeoff between simplicity and performance | 15:52 |
dmsimard | i.e, one large API call that returns everything vs one small API call that returns only what you care about | 15:53 |
dmsimard | If we keep the UI in 1.0 fairly similar to what we currently have in 0.x: http://logs.openstack.org/82/610382/1/check/tox-format/06b3af2/ara-report/ | 15:54 |
dmsimard | I could see the actual tasks only being loaded when I click on "tasks", for example | 15:54 |
apollo13 | jupp | 15:55 |
dmsimard | right now in 0.x, everything is loaded although there are some clever hacks to shift some of that to be asynchronous or just in time | 15:55 |
dmsimard | like iframes inside modals | 15:55 |
dmsimard | ¯\_(ツ)_/¯ | 15:55 |
apollo13 | hihi | 15:56 |
apollo13 | dmsimard: ok, did a first stab at it in https://review.openstack.org/#/c/610635/ -- integration tests obviously fail. does gerrit/zulu have a good idea on how to handle that now? I mean the reference to the ara-plugins@master will prevent this from working till ara-plugins is fixed… | 16:12 |
apollo13 | can I somehow reference another version or will we just merge as is and retest later… | 16:12 |
apollo13 | gotta see if I can fix it locally, lets see | 16:13 |
dmsimard | apollo13: the intent is that the integration jobs should take the components from source if they have been prepared by zuul | 16:17 |
dmsimard | Zuul allows you to declare that a patch depends on another patch in another project | 16:17 |
dmsimard | So you can make an API change that implements a new feature, and then create your callback patch that uses the new feature | 16:19 |
apollo13 | ok, let me do that and lets see how we can link it up :) | 16:19 |
dmsimard | I need to look at how we can make "ansible-integration" actually do this | 16:20 |
dmsimard | Because the dependencies can go both ways | 16:20 |
apollo13 | ok, got you https://review.openstack.org/#/c/610635/ and https://review.openstack.org/#/c/610640/ for now | 16:24 |
apollo13 | I think ara-plugins will still be green since it doesn't require ara-server changes | 16:24 |
apollo13 | so if that passes you can merge it and retest the server changes | 16:24 |
*** sshnaidm_ is now known as sshnaidm|afk | 16:24 | |
apollo13 | gotta take a break, still on sick leave… | 16:25 |
dmsimard | apollo13: all good, please take care of yourself :) | 16:43 |
dmsimard | apollo13: huh, looks like we were already passing the playbook id when creating the task https://github.com/openstack/ara-plugins/blob/master/ara/plugins/callback/ara_default.py#L209 | 17:20 |
dmsimard | I guess it was just silently discarded | 17:21 |
*** tbielawa is now known as tbielawa|lunch | 17:27 | |
*** bcoca has joined #ara | 18:10 | |
*** sshnaidm|afk has quit IRC | 19:06 | |
*** tbielawa|lunch is now known as tbielawa | 19:16 | |
*** themurph has quit IRC | 20:23 | |
*** tbielawa has quit IRC | 20:37 | |
*** bcoca has quit IRC | 22:30 | |
*** openstackgerrit has joined #ara | 23:58 | |
openstackgerrit | Merged openstack/ara-plugins master: Pass playbook along during result creation. https://review.openstack.org/610640 | 23:58 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!