App crashes in prometheus collector on Node stop action: 'Node' object has no attribute 'uid'

Every time when the app tries to stop a node it will crash in prometheus collector, because object Node has no attribute uid.
It seems that pod object has this attribute, but not node:
https://github.com/bloomberg/powerfulseal/blob/f3014fa75477b1f123e87fa12df8e76491e3cd2a/powerfulseal/clouddrivers/aws_driver.py#L30-L39:
def create_node_from_server(server, ip):
    """ Translate AWS EC2 Instance representation into a Node object.
    """
    return Node(
        id=server.id,
        ip=ip,
        az=server.placement['AvailabilityZone'],
        name="",
        state=server_status_to_state(server.state),
)

Error: (removed actual values, note that name doesn't have a value, and also there is no field uid)
Filtered set length: 1
Acting on these: [[node no=xxx id=<ID> ip=xx.xx.xx.xx az=ZONE groups=['ARCH', 'AZ', 'INSTSANCE_TYPE', 'NODE_LABEL', 'ip-xx-xx-xx-xx', 'OS', 'node'] name= state=NodeState.UP]]
Action stop on [node no=xxx id=<ID> ip=xx.xx.xx.xx az=ZONE groups=['ARCH', 'AZ', 'INSTSANCE_TYPE', 'NODE_LABEL', 'ip-xx-xx-xx-xx', 'OS', 'node'] name= state=NodeState.UP]
Traceback (most recent call last):
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/policy/node_scenario.py", line 61, in action_stop
    self.metric_collector.add_node_stopped_metric(item)
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/metriccollectors/prometheus_collector.py", line 73, in add_node_stopped_metric
    NODE_STOPS.labels(STATUS_SUCCESS, node.uid, node.name).inc()
AttributeError: 'Node' object has no attribute 'uid'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/bin/powerfulseal", line 11, in <module>
    load_entry_point('powerfulseal', 'console_scripts', 'powerfulseal')()
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/cli/__main__.py", line 391, in start
    main(sys.argv[1:])
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/cli/__main__.py", line 387, in main
    metric_collector=metric_collector)
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/policy/policy_runner.py", line 88, in run
    scenario.execute()
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/policy/scenario.py", line 65, in execute
    self.act(filtered_set)
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/policy/node_scenario.py", line 90, in act
    return self.act_mapping(items, actions, mapping)
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/policy/scenario.py", line 221, in act_mapping
    method(item, params)
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/policy/node_scenario.py", line 64, in action_stop
    self.metric_collector.add_node_stop_failed_metric(item)
  File "/root/.local/share/virtualenvs/opt-zvmYt2-H/src/powerfulseal/powerfulseal/metriccollectors/prometheus_collector.py", line 76, in add_node_stop_failed_metric
    NODE_STOPS.labels(STATUS_FAILURE, node.uid, node.name).inc()

Pipefile:
[[source]]
url = "https://pypi.python.org/simple"
verify_ssl = true
name = "pypi"

[packages]
powerfulseal = {git = "git://github.com/bloomberg/powerfulseal.git", editable = true, ref = "master"}
Pipfile = "*"

[dev-packages]

[requires]
python_version = "3.6"

Start command:
pipenv run powerfulseal --inventory-kubernetes --aws-cloud --kube-config ~/.kube/config  --prometheus-collector --prometheus-host '<IP_ADDRESS>' --prometheus-port 8080  --remote-user core --ssh-allow-missing-host-keys --verbose -v --run-policy-file chaos_policy.yml

If prometheus is not enabled the app works fine

I've knocked this out through the UI (away from keyboard), but the CI seems to be happy: https://github.com/bloomberg/powerfulseal/pull/95 - check it out when you have a moment 😄

thank you for addressing this promptly


What should m.redraw do when called from a lifecycle method

Current behavior is undefined (and documented to be undefined). Typically, it overflows stack
@isiahmeadows' suggestions are to 1) throw, 2) rollback-and-restart or 3) request an async redraw

@barneycarroll has objections

@isiahmeadows points out this is highly non-trivial due to rollback semantics

this is similar to what React's setState does (see: http://thereignn.ghost.io/on-the-async-nature-of-setstate-in-react/ ). My concern is sometimes-async semantics is highly surprising and confusing

Other points to consider: allowing a mechanism for data to modifiable from a lifecycle method (which is by definition within the call graph of a render call) means that calls to render can no longer be guaranteed to keep state stable (think Angular's infdig).
I have been considering adding m.redraw.sync, which would make it easier to reason about the synchronicity semantics of the redraw irrespective of whether it's in a lifecycle method or not (but one would still need to be aware of the undefined semantics of synchronous redraws within lifecycle methods if they chose to use m.redraw.sync for whatever reason).

In v0, a call to redraw during the draw lifecycle is a no-op. That makes perfect sense up till the point of DOM persistence — so IMO calls to redraw invoked during oninit, onbeforeupdate, onbeforeremove & view should follow this behaviour. As regards oncreate / onupdate, since these occur after Mithril has reconciled the internal vnode graph, I don't see why the call shouldn't be respected.
Worth pointing out that with the advent of stream-like props, we will likely see users create streams in component initialisers and potentially write to those streams or their derivatives downstream. Further derivatives may well set up conditions to redraw on stream update. In this scenerio one can imagine redraw being spammed during a view tree's initialisation, and all author intentions being met since the streams outcomes will have resolved before view execution. In this sense, silent redraw no-op during view composition becomes more of a feature in v1, as opposed to a code smell as it might have been in v0.

I'd also like to point out that with 2 (rollback), there is also the risk of leaving user components in an invalid state, which is generally a bad idea.

@lhorie With 3, I feel it may be a necessity to resort back to the async redraw semantics, the way this is going. It previously timed itself to use requestAnimationFrame unless called in a way that forces a sync redraw.
As for the API, I would strongly prefer a separate sync method, because then you can just do stream.map(m.redraw), knowing it's just going to run asynchronously regardless of what's in the stream, or m.redraw.sync(), knowing it's always synchronously.

Yeah, can we just drop the rollback, draw during draw idea? We've stated again and again that it's an implementation nightmare, nobody has asked for it, and nobody can work out how or why anybody might want it. Would be fantastic if we could just not bring it up again unless there's new information.
@isiahmeadows the idea of a separate method for sync is a very good one. I support that proposal whole heartedly. Making redraw semantics more explicit can only be a good thing.

@lhorie To clarify, my async-redraw idea would be to return a single-update stream. And it is entirely possible we could get away with not having a sync redraw, although the testing might become a little more complex.
@barneycarroll You could, in theory, disrupt a running redraw by running a sync one right in the middle of it, but Mithil could (and should IMHO) throw an error in that case to make it very clear it's not okay to do that. Async redraws won't have that problem, though.

@isiahmeadows dunno, I think a no-op plus warning would do. Assume it's an incidental accident and move on, right? The case for redraw calls within a draw is setting up a prop redraw dependency, which could trigger upon initialisation. These redraw requests could well be synchronous, but I still wouldn't force the author to rewrite their init code to avoid immediate trigger, which I think we can still safely call a no-op — with a warning, in case the developer is actually expecting this scenario to do something — but the eagerness to presume author error and throw isn't justified IMO.

If it's really intentional, you can always guard it with a try-catch and
ignore all errors (I've done that before with other things). I'm suggesting
throwing since it's likely to be an author error.
On Sat, Jul 23, 2016, 13:58 Barney Carroll<EMAIL_ADDRESS>wrote:

@isiahmeadows https://github.com/isiahmeadows dunno, I think a no-op
plus warning would do. Assume it's an incidental accident and move on,
right? The case for redraw calls within a draw is setting up a prop redraw
dependency, which could trigger upon initialisation. These redraw requests
could well be synchronous, but I still wouldn't force the author to rewrite
their init code to avoid immediate trigger, which I think we can still
safely call a no-op — with a warning, in case the developer is actually
expecting this scenario to do something — but the eagerness to presume
author error and throw isn't justified IMO.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/lhorie/mithril.js/issues/1166#issuecomment-234731508,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AERrBKOEk7kVpVBY1e0v3OWvdGoAPkJCks5qYlZFgaJpZM4JQyIs
.

I don't know where this contention of likelihood is coming from, but I think the pattern I described is credible. I don't think people should be made to write try catches in application code to get around framework obnoxiousness, and I don't see what the benefits of error throwing are.

What would be a use case of calling a synchronous redraw while another
redraw is being run?
On Sat, Jul 23, 2016, 14:09 Barney Carroll<EMAIL_ADDRESS>wrote:

I don't know where this contention of likelihood is coming from, but I
think the pattern I described is credible. I don't think people should be
made to write try catches in application code to get around framework
obnoxiousness, and I don't see what the benefits of error throwing are.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/lhorie/mithril.js/issues/1166#issuecomment-234732020,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AERrBB_KhmVYoscm_ip_U4UcaO3G0Xpuks5qYli8gaJpZM4JQyIs
.

@isiahmeadows
var MyComp = {
  oncreate: function(vnode) {
    this.top = (document.body.offsetHeight - vnode.dom.offsetHeight) / 2
    m.redraw(true) //trying to avoid flickering
  },
  view: function(vnode) {
    return m("div", {style: {top: this.top}}, vnode.children)
  }
}

@lhorie @barneycarroll
Okay. Here's my idea to try to mitigate the problem:

m.redraw() - Schedule a requestAnimationFrame-guarded redraw that is always async.
Returns a stream that is resolved with undefined and ended on completion, and the redraw request, if pending, can be aborted by ending the stream early.

m.redraw.current() - Schedule a redraw in the current frame. If we're currently redrawing, then another redraw is requested. Otherwise, it starts redrawing immediately.
Returns a stream that is resolved with undefined and ended on completion, and the redraw request, if pending, can be aborted by ending the stream early.

m.redraw.redrawing() - Whether we are in the middle of a redraw.

m.redraw.pending() - Whether any other redraws are pending in subsequent frames.

m.redraw.pendingCurrent() - Whether any other redraws are pending in the current frame.

Yes, m.redraw.current() is sometimes async, sometimes not, but that's mostly unavoidable unless you're okay with scheduling a microtask (I'll note that a robust polyfill exists, but it's non-trivial).

I don't think that level of complexity addresses any of the conceivable use cases we've described, it just makes the API surface harder to grapple with. I do think:

The separate m.redraw.sync method is a really good idea (that needs its own issue, since it's about API surface, not implementation).
Invoking a sync redraw during virtual DOM build (ie anywhere between the first oninit and the last view ) should produce an error, since it isn't possible or desirable to fulfil the author's explicit intention.
Invoking an async redraw during virtual DOM build should silently noop since it can safely be assumed that whatever state change triggered it has taken place and therefore will be persisted to the view.
Calls for sync or async redraw post-build (ie during oncreate, onupdate) should be allowed since it doesn't pose any implementation problems.

@barneycarroll re. implementation issues: currently, m.redraw()has as many render calls as there are mount points, run in the chronological mounting order. So a synchronous redraw called from a post-build hook in all but the last render would end up being sandwiched in between the render calls of the previous redraw, which may be surprising.
render_1(root_1)
    render_2(root_1)
    render_2(root_2)
render_1(root_2)

Here's a little tree with nested components, and couple of mountpoints with lifecycle logs. This kind of stuff becomes really hard to rationalise without reference :)
@pygy multi-tenancy is a rare enough scenario that this shouldn't be a common gotcha (and honestly, it's difficult to work out how that gotcha would manifest itself in terms of author expectation disparity).
The quick way to caveat my list above is to say that sync draws are actually banned during the entire draw loop.
What is the use case for an immediate post draw synchronous redraw? Let's not forget that requestAnimationFrame is a feature. @lhorie's example — inspecting DOM side-effects in order to inform the view model, which other parts of the current view depend on — is the only credible scenario for this I can imagine (I've been there). In this scenario, the author should be grateful for the internal requestAnimationFrame, rather than forcing the browser to recompute view despite its own stated inability to do so. If the author is afraid of flicker (we're talking about a situation where your implementation mandates 2 animation frames in order to achieve what would otherwise conceptually be a single frame), then we can assume — given they are adept and engaged enough to isolate relevant DOM props and accommodate this into their view model, and understanding the lifecycle loop well enough to put all this together — that they are adept and engaged enough to go the extra mile and do whatever necessary temporary display:hidden magic and extend their view model more comprehensively to cater for the fact that due to limitations in DOM, they are effectively dealing with a draw in 2 steps.
…
This conversation is so reminiscent of those we've had in the past around controller-based m.redraw.strategy( 'diff' ), m.startComputation(), { background : true } that I'm increasingly convinced synchronous redraws are just a bad API. Shall we at least wait until somebody presents a use case born out of actual need, and just leave it out until then? It can always be patched in easily enough in 1.X if and when people really want it.

@barneycarroll

What is the use case for an immediate post draw synchronous redraw?

I asked the same question, and here's the answer. It's not the obvious default behavior, so I intentionally proposed it to be its own method.
As for how it could be improved? I'd directly set vnode.dom.style.top instead of passing it through the vdom tree. It's easier and faster to manually diff that yourself.

@barneycarroll

What is the use case for an immediate post draw synchronous redraw?

I asked the same question, and here's the answer. It's not the obvious default behavior, so I intentionally proposed it to be its own method.
As for how it could be improved? I'd directly set vnode.dom.style.top instead of passing it through the vdom tree. It's easier and faster to manually diff/patch that yourself (using onupdate). So IMHO that example isn't a good one.
If you need to do things like that higher up a tree, you shouldn't be passing low-level details like that around at a high level. Instead, you should still be diffing the relevant properties at a lower level in onupdate.
I agree that the use case isn't there anymore.

@lhorie @barneycarroll Here's my new suggestion:
m.redraw() schedules a redraw on the next requestAnimationFrame, and is always sync. It returns a stream that is resolved with undefined and ended on completion, and the redraw request, if pending, can be aborted by ending the stream early. If multiple requests are made within the same frame, it simply batches calling those into the next frame.
This should at least fix the problem. Also, note that m.render() will (and should) remain synchronous.

Interesting (and possibly unrelated) side note: I believe you could almost literally create custom components that don't depend on Mithril's attribute diffing:
var div = {
  oncreate: function (vnode) {
    vnode.dom = document.createElement("div")
    // set initial attrs
  },
  onupdate: function (vnode) {
    // update attrs
  },
  view: function () {
    // here's the only catch...managing this
    return []
  },
}

@isiahmeadows agreed with m.redraw() being always async, on the next frame. Otherwise we end up releasing Z̮̤͎̗̯̳ͬͭ̉ͮͭͅȁ̮̬͇̯̞̣̼̓l̲̭̫̗̝̉̄̍͒͑̇̽g̯̘̝ͤͨŏ̥.
Your stream-based idea also does make sense. I don't know why you'd want to cancel a pending redraw, but it would provide an onredraw hook too, which has been requested in the past.
Your diff-less scheme can be made to work. Return a single element in the view, replace it in oncreate and skip diff with onbeforeupdate(vnode, old) {return false} (where you can access old.dom).

@pygy
BTW, if you close a stream, it's permanently closed.
But I agree such a hook would also be useful, but on a separate method. You could also use that for error handling: if an error occurs while rendering, you can emit an error event instead of just logging it to the console, leaving it uncaught.
Here's my current idea:

var stream = m.redraw() - Redraws asynchronously and emits a single undefined when done, or the error if one occurred when rendering. Closes immediately afterwards. If a redraw request is already pending, this will piggyback off of that one.
var stream = m.onredraw() - Returns a stream that emits undefined on each redraw.

The redraw hook thing is a bit redundant because all idiomatic application code in Mithril is already structured into components, and all the code in components exists within lifecycle hooks — which are triggered by redraws. If you want to call a redraw and follow up with further actions when it resolves, you set a flag in component state and call your redraw; onbeforeupdate (or wherever suits), you sniff for said flag. The lifecycle hooks are the callbacks. So a canonical example looks like this:
const Square = {
  oncreate : vnode => {
    vnode.state.height = vnode.dom.clientWidth

    Promise.resolve().then( m.redraw )
  },

  view : vnode =>
    m( '.Square', {
      style : {
        display : 'inline-block',
        height  : vnode.state.height + 'px'
      }
    },
      vnode.children
    )
}

@barneycarroll You may have a point. Here's an idea, have a way to capture errors during lifecycle hooks, and observe them. That's very useful for things like error handling in production.
Here's my new two methods

var stream = m.redraw() - same as above: redraw, and emit once when done.
var errorStream = m.redraw.error() - stream of errors thrown during component lifecycle events

var MyComp = {
  oncreate: function(vnode) {
    this.top = (document.body.offsetHeight - vnode.dom.offsetHeight) / 2
    m.redraw(true) //trying to avoid flickering
  },
  view: function(vnode) {
    return m("div", {style: {top: this.top + "px"}}, vnode.children)
  }
}

@lhorie That scenario could be solved without a redraw() by using a style vnode whose content is set dynamically during the oncreate phase.
var MyComp = {
  oncreate: function(vnode) {
    this.top = (document.body.offsetHeight - vnode.dom.offsetHeight) / 2
  },
  view: function(vnode) {
    return m("#MyComp", [
      m('style', {
        oncreate: function(styleVnode) {
          styleVnode.dom.styleSheet.cssText = "#MyComp {top: " + vnode.state.top + "px}"
        },
        onbeforeupdate: function(){return false}
      }, ""),
      vnode.children
    ])
  }
}

Also, even for single tenant apps, calling redraw(true) can cause onupdate to fire before oncreate.
m.mount(root, {view: function() {
  return [
    m("div", {oncreate: m.redraw.bind(null, true)}),
    m(".problem-here", {
      oncreate: console.log.bind(console, "oncreate"),
      onupdate: console.log.bind(console, "onupdate")
    })
  ]
}})

That could be worked around by having a single, global event queue shared between all roots, but it complicates the implementation quite a bit.

Globally, making render() atomic for a given root seems like the best course of action (by setting a flag on the root). Whether to throw or to noop is another story.
If a use case ever requires it, a synchronous post-redraw hook could be provided to schedule a subsequent sync redraw.

I updated the docs to indicate m.redraw is to be considered asynchronous even if in some occasions they do run synchronously for perf reasons.

@lhorie It'd be better if you document when it's synchronous and when it's asynchronous. If you're going to keep it sometimes-synchronous, that's definitely something that must be clearly noted, to prevent a whole host of confusion later on.


python -m pip issue

Dear Mike:
Thanks for such a great piece of software. I discovered it a few weeks ago, and I am using it to dump (tentatively, roughly) microfilms from familysearch.org to text, that have not yet been indexed (maybe never).
Thanks to the program of you can save months of turning the page trying to read old writing trying to find a last name.
I do not know if it corresponds, but I comment that the 1st time when trying to install, as the documentation indicates:
python3 -m pip install git+https@github.com:caltechlibrary/handprint.git --user --upgrade
I got the following error.
"ERROR: Invalid requirement: 'git+https@github.com:caltechlibrary/handprint.git'
Hint: It looks like a path. File 'git+https@github.com:caltechlibrary/handprint.git' does not exist."
In my case it does work:
python -m pip install -e git+https://github.com/caltechlibrary/handprint.git@master#egg=handprint  --user --upgrade
, with:
git version 2.25.1
Python 3.8.1
pip 19.3.1
Again thank you very much!
Cheers!

Thank you for reporting this. I checked it in my own environment, and sure enough, it fails to work with the same error. This is very puzzling because I've used basically the same installation instructions for a bunch of programs and others fail too. Maybe something changed in how GitHub handles git urls?  Anyway, I'll update the docs as soon as I can figure out what's wrong.

The URL mentioned in the README is now a corrected version of the original. It's a little bit simpler than the one you ended up using. It works in my environment, but if it fails for you, please let me know so that I correct it (again).


Is there any way in which we can only display the tags?

Right now, if I set a taglist, it displays the tags but also adds a cursor along with it to add new tags, which is great. Another use case for this plugin is to only display tags. Is this feature already present? If not I'll be happy to work on it and make a pull request

Thanks,There is a simple way out is to remove EditText view, which is the
last child view of FlowLayout. Wellcome to do a pull request
On Thu, Jul 7, 2016 at 6:21 AM Shaishav Gandhi<EMAIL_ADDRESS>wrote:

Right now, if I set a taglist, it displays the tags but also adds a cursor
along with it to add new tags, which is great. Another use case for this
plugin is to only display tags. Is this feature already present? If not
I'll be happy to work on it and make a pull request
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/qiugang/EditTag/issues/1, or mute the thread
https://github.com/notifications/unsubscribe/AGglr6X62pm_k-B9IqAGPoIvygv-kqofks5qTCpagaJpZM4JGj_O
.