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

master_tops appears broken in 2014.1.5 #13673

Closed
mgwilliams opened this issue Jun 24, 2014 · 11 comments
Closed

master_tops appears broken in 2014.1.5 #13673

mgwilliams opened this issue Jun 24, 2014 · 11 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@mgwilliams
Copy link
Contributor

On 2014.1.5, custom tops modules (located in {extension_modules}/tops) are loaded (__virtual__ function is called), but not utilized (top function is not invoked).

The tops module (/srv/salt/modules/tops/test.py):

import logging


log = logging.getLogger(__name__)


def __virtual__():
    log.warning('test loaded')
    return 'test'


def top(**kwargs):
    log.warning('test top')
    return {'base': ['test']}

master config:

extension_modules: /srv/salt/modules/
master_tops:
  test: True
salt-call state.show_top

On 2014.1.4:

local:
    ----------
    base:
        - test

On 2014.1.5:

local:
    ----------
@thatch45 thatch45 added the Bug label Jun 24, 2014
@thatch45 thatch45 added this to the Approved milestone Jun 24, 2014
@thatch45
Copy link
Contributor

Marked as High Severity, we will look into this ASAP

@cro
Copy link
Contributor

cro commented Jun 24, 2014

@thatch45 ref #13686

@pass-by-value
Copy link
Contributor

I did a git bisect and found that this was introduced by 76fdca0

Looking to see if the changes mentioned by #13686 can get us back to the expected behavior.

@cachedout
Copy link
Contributor

We should get regression testing here ASAP.

On Tue, Jun 24, 2014 at 2:47 PM, Aditya Kulkarni notifications@github.com
wrote:

I did a git bisect and found that this was introduced by 76fdca0
76fdca0

Looking to see if the changes mentioned by #13686
#13686 can get us back to the
expected behavior.


Reply to this email directly or view it on GitHub
#13673 (comment).

@pass-by-value
Copy link
Contributor

@cachedout Good idea! I'll add a regression test to it.

@cro cro modified the milestones: ww27, Approved Jun 25, 2014
@mgwilliams mgwilliams assigned cro and unassigned pass-by-value Jun 25, 2014
@cro cro assigned cro and unassigned pass-by-value Jun 25, 2014
@pass-by-value
Copy link
Contributor

I've added #13762 and #13730 to test this

@pass-by-value
Copy link
Contributor

Also added #13764

@pass-by-value
Copy link
Contributor

#13807 is added so that tests on 2014.1 branch are passing

@rallytime
Copy link
Contributor

Awesome!

@pass-by-value
Copy link
Contributor

#13940

@basepi
Copy link
Contributor

basepi commented Jul 3, 2014

This is finally fixed! The new test is passing and the fix is in both 2014.1 and develop, and will be in 2014.1.6.

@basepi basepi closed this as completed Jul 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests

7 participants