Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot create executor-driver with expected initialization #8

Closed
oubiwann opened this issue Jun 9, 2016 · 4 comments
Closed

Cannot create executor-driver with expected initialization #8

oubiwann opened this issue Jun 9, 2016 · 4 comments

Comments

@oubiwann
Copy link
Member

oubiwann commented Jun 9, 2016

When attempting to create a mesomatic executor driver like so:

(require [clojure.core.async :as a]
         [mesomatic.async.executor :as async-executor]
         [mesomatic.executor :as executor])
(def ch (a/chan))
(def exctr (async-executor/executor ch))
(executor/executor-driver exctr)

the following error occurs:

IllegalArgumentException Don't know how to create ISeq from: 
  mesomatic.executor$wrap_executor$reify__9882  
  clojure.lang.RT.seqFrom (RT.java:542)

Double-checking this, I found that:

  • org.apache.mesos.MesosExecutorDriver.java expects an instance of org.apache.mesos.Executor during init
  • mesomatic.executor/executor-driver seems to expect a map that
    • is then converted to protobuff data
    • and passed to MesosExectorDriver's init
@oubiwann
Copy link
Member Author

oubiwann commented Jun 9, 2016

I don't have a complete understanding of mesomatic internals yet, and I'm not sure how closely its behaviours are aiming to match the Java lib's, but:

  • it seems that instead of ExecutorInfo getting passed to MesosExectorDriver
  • the result of (in the async case) (async-executor/executor ch) should get passed

@oubiwann
Copy link
Member Author

oubiwann commented Jun 9, 2016

I've just changed the mesomatic in my project's checkouts dir to remove the conversion of the input to protobuffs ... in otherwords, following the same approach used by the Java lib:

(defn executor-driver
  [executor]
  (let [d (MesosExecutorDriver. executor)]
    (reify ExecutorDriver
      (abort! [this]
        (pb->data (.abort d)))
      (join! [this]
        (pb->data (.join d)))
      (run-driver! [this]
        (pb->data (.run d)))
      (send-framework-message! [this data]
        (pb->data (.sendFrameworkMessage d data)))
      (send-status-update! [this status]
        (pb->data (.sendStatusUpdate d (data->pb status))))
      (start! [this]
        (pb->data (.start d)))
      (stop! [this]
        (pb->data (.stop d))))))

Using this in the REPL produced a successful driver creation:

(def ch (chan))
(def exctr (async-executor/executor ch))
(executor/executor-driver exctr)
#object[mesomatic.executor$executor_driver$reify__9890 0x6fe33722 
  "mesomatic.executor$executor_driver$reify__9890@6fe33722"]

However! I haven't tried to use this yet, and I have no idea if this change impacts anything else in this library (or projects using this library that might expect the existing behaviour). I will submit a PR for this small change which can get updated according to your feedback/corrections.

oubiwann added a commit to oubiwann/mesomatic that referenced this issue Jun 9, 2016
This is a risky change, as other code/projects may depend upon the old
behaviour.

This addresses issue clojusc#8, but may need more work (depending upon impact).
oubiwann added a commit to oubiwann/mesomatic that referenced this issue Jun 9, 2016
This is a risky change, as other code/projects may depend upon the old
behaviour.

This addresses issue clojusc#8, but may need more work (depending upon impact).
@oubiwann
Copy link
Member Author

oubiwann commented Jun 9, 2016

Quick note: such that it is, my sample command-line executor (using lein) is now successfully completing (with this change in place) whereas before it was not. Still need to do more testing on the framework itself to further assess impact ...

oubiwann added a commit to oubiwann/mesomatic-hello that referenced this issue Jun 9, 2016
oubiwann added a commit to oubiwann/mesomatic that referenced this issue Jun 14, 2016
This is a risky change, as other code/projects may depend upon the old
behaviour.

This addresses issue clojusc#8, but may need more work (depending upon impact).
oubiwann added a commit to oubiwann/mesomatic that referenced this issue Jun 14, 2016
This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue clojusc#8, but may need more work (depending upon impact).
pyr pushed a commit that referenced this issue Jun 21, 2016
#9)

* Mirror the behaviour of the Java library.

This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue #8, but may need more work (depending upon impact).

* Updated method names to match current Proto.java.

* Fixed typo (:driver -> driver) in reregistered.
pyr pushed a commit that referenced this issue Jun 21, 2016
* Mirror the behaviour of the Java library.

This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue #8, but may need more work (depending upon impact).

* Updated method names to match current Proto.java.

* Fixed typo (:driver -> driver) in reregistered.

* Added import for operation and its constants.

* Added pb->data for Operation types.

* Added Operatoin record and pb->data method.

* Added pb->data for operation types.

* Finished first draft of #11.

* Added missing imports.
@oubiwann
Copy link
Member Author

The fix for this has been merged 😄

pyr pushed a commit that referenced this issue Aug 31, 2016
* Mirror the behaviour of the Java library.

This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue #8, but may need more work (depending upon impact).

* Updated method names to match current Proto.java.

* Fixed typo (:driver -> driver) in reregistered.

* Added import for operation and its constants.

* Added pb->data for Operation types.

* Added Operatoin record and pb->data method.

* Added pb->data for operation types.

* Finished first draft of #11.

* Added missing imports.

* Added accept-offers method.
oubiwann added a commit to oubiwann/mesomatic that referenced this issue Sep 8, 2016
This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue clojusc#8, but may need more work (depending upon impact).
pyr pushed a commit that referenced this issue Sep 9, 2016
* Mirror the behaviour of the Java library.

This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue #8, but may need more work (depending upon impact).

* Updated method names to match current Proto.java.

* Fixed typo (:driver -> driver) in reregistered.

* Added missing TaskStatus reason constants.
pyr pushed a commit that referenced this issue Sep 9, 2016
* Mirror the behaviour of the Java library.

This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue #8, but may need more work (depending upon impact).

* Updated method names to match current Proto.java.

* Fixed typo (:driver -> driver) in reregistered.

* Fixed the direction of conversion.

Also updated the method name for getting object attributes.

* Status updates take (->pb :TaskStatus ...), so no need for conversion.

There may be a better way to do that, though ...
pyr pushed a commit that referenced this issue Sep 9, 2016
* Mirror the behaviour of the Java library.

This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue #8, but may need more work (depending upon impact).

* Updated method names to match current Proto.java.

* Fixed typo (:driver -> driver) in reregistered.

* Added missing source fields for data->pb.
pyr pushed a commit that referenced this issue Sep 9, 2016
* Mirror the behaviour of the Java library.

This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue #8, but may need more work (depending upon impact).

* Updated method names to match current Proto.java.

* Fixed typo (:driver -> driver) in reregistered.

* Added support for implicit-acknowledgements argument.
pyr pushed a commit that referenced this issue Sep 9, 2016
* Mirror the behaviour of the Java library.

This may be a risky change, as other code/projects could theoretically depend
upon the old behaviour (I'm pretty sure it was broken, regardless).

This addresses issue #8, but may need more work (depending upon impact).

* Updated method names to match current Proto.java.

* Fixed typo (:driver -> driver) in reregistered.

* Added acknowledge-status-update and suppress-offers methods.

* Fixed bad arg (should be the driver, not the implementation).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant