foreman_docker icon indicating copy to clipboard operation
foreman_docker copied to clipboard

Fixes #8409 - Pull docker image asynchronously

Open dLobatog opened this issue 10 years ago • 16 comments

dLobatog avatar Jan 23 '15 10:01 dLobatog

Updated to let the action be a good thread citizen and not block the pool. To be tested with foreman core develop, I'll submit a patch with 1.7 compat fixes separately so we can keep track of these.

cc @witlessbird @daviddavis @parthaa can you test it?

dLobatog avatar Jan 28 '15 11:01 dLobatog

Updated with a test for the provision plan and commented out the tests for the run phase of start and pull. I'll work with @iNecas or @pitr-ch trying to figure out what I'm doing wrong so that these tests don't pass.

dLobatog avatar Jan 29 '15 14:01 dLobatog

re the test failure, what appears to be happening is that foreman-tasks is registering its extensions to the test rake task from its engine.rb initialisers. foreman-docker is of course requiring foreman-tasks, but the 'test' task(s) are now extended with both foreman-docker's and foreman-tasks' tests.

I don't know whether we should be running foreman-tasks' tests from here or not. We run core tests from plugins to check a plugin didn't break a core behaviour, and you could argue whether that should be the case or not for a plugin building on another plugin. It might be harder, since plugin tests may require additional development dependencies.

If you do want to run foreman-tasks tests, then I think something's wrong with the load paths (here?).

If not, then some refactoring of how rake tasks are modified (e.g. so they only happen when testing the plugin itself) or how we choose which sets are run is needed.. I need to think a bit more about that I guess.

domcleal avatar Feb 03 '15 10:02 domcleal

This PR should address the issue with the foremant-tasks tests (so that the tasks tests should not be run as part of this engine tests) https://github.com/theforeman/foreman-tasks/pull/103

iNecas avatar Mar 13 '15 10:03 iNecas

@iNecas Awesome! Any prospective dates for a release?

dLobatog avatar Mar 15 '15 16:03 dLobatog

Hopefully today, we have other feature that I would like to get in before the release, but if we don't get that done till tomorrow, i will release new version without it anyway.

iNecas avatar Mar 16 '15 10:03 iNecas

The new versions in a PR for foreman-packaging https://github.com/theforeman/foreman-packaging/pull/582

iNecas avatar Mar 17 '15 12:03 iNecas

[test]

iNecas avatar Mar 17 '15 12:03 iNecas

[test]

dLobatog avatar Mar 23 '15 11:03 dLobatog

[test] this got aborted I think

dLobatog avatar Mar 23 '15 14:03 dLobatog

[test] - tests were failing because I was using minitest-reporters here (and it's not used in Foreman core)

dLobatog avatar Apr 08 '15 06:04 dLobatog

@daviddavis @parthaa @witlessbird Could you give a last test at this before merging?

dLobatog avatar Apr 08 '15 07:04 dLobatog

looks good to me.

dmitri-d avatar Apr 09 '15 11:04 dmitri-d

I've been testing this a bit more thoroughly myself and I think this should:

  • Show a link to the task created when creating a container, telling the user to check Monitor > Tasks is not very user friendly.
  • If the task fails, show in the show container page that the task in question failed

I'll polish it up with these changes & resubmit when I get a chance.

dLobatog avatar Apr 12 '15 13:04 dLobatog

@dLobatog any updates?

daviddavis avatar Dec 22 '15 17:12 daviddavis

I'll pick this up again when ActiveJob is available

dLobatog avatar Mar 09 '17 15:03 dLobatog