You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by GitBox <gi...@apache.org> on 2021/08/16 07:53:48 UTC

[GitHub] [mesos] andreaspeters opened a new pull request #405: Add framework plugin

andreaspeters opened a new pull request #405:
URL: https://github.com/apache/mesos/pull/405


   As I mentioned, I add a framework plugin to mesos-cli and some new features for the already existing task plugin.
   
   The features are:
   
   - show all framework
   - inspect frameworks and tasks
   - disable the limitation of tasks in the task list


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] andreaspeters commented on a change in pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
andreaspeters commented on a change in pull request #405:
URL: https://github.com/apache/mesos/pull/405#discussion_r701430413



##########
File path: src/python/cli_new/lib/cli/http.py
##########
@@ -64,7 +65,7 @@ def read_endpoint(addr, endpoint, config, query=None):
                            .format(url=url, error=str(exception)))
 
 
-def get_json(addr, endpoint, config, condition=None, query=None):
+def get_json(addr, endpoint, config, query=None):

Review comment:
       Not the mesos_cli. I had a look. But of course, I will also feel better u you keep an eye on it. :-)  




-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] andreaspeters commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
andreaspeters commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-899649417


   :joy: Take your time and enjoy your holiday. 


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] cf-natali commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
cf-natali commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-980316967


   @andreaspeters I'm closing this one since you've split it into separate MRs.


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] cf-natali closed pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
cf-natali closed pull request #405:
URL: https://github.com/apache/mesos/pull/405


   


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] andreaspeters edited a comment on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
andreaspeters edited a comment on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-912429455


   Shell I open three PR's? Technically  there are three kind of changes.
   
   1. Add Framework Plugins
   2. Update Task Plugins
   3. [x]  Update mesos_cli lib.
   
   What do u think?


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] cf-natali commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
cf-natali commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-899592853


   Hey,
   
   I'm on holiday so I'll have a look when I'm back, unless someone else beats me to it.


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] andreaspeters commented on a change in pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
andreaspeters commented on a change in pull request #405:
URL: https://github.com/apache/mesos/pull/405#discussion_r701423812



##########
File path: src/python/cli_new/lib/cli/http.py
##########
@@ -37,6 +37,7 @@ def read_endpoint(addr, endpoint, config, query=None):
 
     try:
         addr = cli.util.sanitize_address(addr)
+

Review comment:
       No reason. :man_shrugging:  will remove it.




-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] cf-natali commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
cf-natali commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-912724891


   Sounds good!


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] cf-natali commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
cf-natali commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-917370232


   @andreaspeters how's the refactoring going? :)


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] andreaspeters commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
andreaspeters commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-912429455


   Shell I open three PR's? Technically  there are three kind of changes.
   
   1. Add Framework Plugins
   2. Update Task Plugins
   3. Update mesos_cli lib.
   
   What do u think?


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] andreaspeters commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
andreaspeters commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-912085063


   > > > I think it would be better to split the changes to the task plugin into its own MR.
   > > 
   > > 
   > > Oh please don't push me to split it. :-( Thats very painfull. There will be more PR's for the mesos-cli in the future. Then I will keep an eye to separate it.
   > 
   > Really?
   > The problem is that fundamentally this MR contains two unrelated changes, so it'd be nice to separate.
   > Either to review, commit, and revert if necessary...
   
   Charles u are killing me. :joy:  But I will do it. 


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] andreaspeters commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
andreaspeters commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-912057976


   > I think it would be better to split the changes to the task plugin into its own MR.
   
   Oh please don't push me to split it. :-( Thats very painfull. There will be more PR's for the mesos-cli in the future. Then I will keep an eye to separate it.   


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] andreaspeters commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
andreaspeters commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-918021712


   > @andreaspeters how's the refactoring going? :)
   
   Didn't found the time the last week. Hopefully the next two weeks. 


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] cf-natali commented on a change in pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
cf-natali commented on a change in pull request #405:
URL: https://github.com/apache/mesos/pull/405#discussion_r701417067



##########
File path: src/python/cli_new/lib/cli/http.py
##########
@@ -64,7 +65,7 @@ def read_endpoint(addr, endpoint, config, query=None):
                            .format(url=url, error=str(exception)))
 
 
-def get_json(addr, endpoint, config, condition=None, query=None):
+def get_json(addr, endpoint, config, query=None):

Review comment:
       Are you sure that nothing uses `condition`?
   
   Also, if you remove t, please remove the mention in the docstring as well:
   ```
   subject to the condition specified in 'condition'. If we are
   ```

##########
File path: src/python/cli_new/lib/cli/http.py
##########
@@ -37,6 +37,7 @@ def read_endpoint(addr, endpoint, config, query=None):
 
     try:
         addr = cli.util.sanitize_address(addr)
+

Review comment:
       Nit: why the newline?




-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] cf-natali commented on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
cf-natali commented on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-912082594


   > > I think it would be better to split the changes to the task plugin into its own MR.
   > 
   > Oh please don't push me to split it. :-( Thats very painfull. There will be more PR's for the mesos-cli in the future. Then I will keep an eye to separate it.
   
   Really?
   The problem is that fundamentally this MR contains two unrelated changes, so it'd be nice to separate.
   Either to review, commit, and revert if necessary...


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mesos] andreaspeters edited a comment on pull request #405: Add framework plugin

Posted by GitBox <gi...@apache.org>.
andreaspeters edited a comment on pull request #405:
URL: https://github.com/apache/mesos/pull/405#issuecomment-912085063


   > > > I think it would be better to split the changes to the task plugin into its own MR.
   > > 
   > > 
   > > Oh please don't push me to split it. :-( Thats very painfull. There will be more PR's for the mesos-cli in the future. Then I will keep an eye to separate it.
   > 
   > Really?
   > The problem is that fundamentally this MR contains two unrelated changes, so it'd be nice to separate.
   > Either to review, commit, and revert if necessary...
   
   Charles u are killing me. :joy:  But I will do it because u are right.


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org