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

Wrong hashing in Flow #436

Closed
JackTemaki opened this issue Jul 27, 2023 · 3 comments
Closed

Wrong hashing in Flow #436

JackTemaki opened this issue Jul 27, 2023 · 3 comments

Comments

@JackTemaki
Copy link
Contributor

JackTemaki commented Jul 27, 2023

During #431 we found that FlowNetworks have incorrect hashing of the post_config entry:

    def __sis_state__(self):
        def get_val(a):
            return get_val(a.get(self)) if isinstance(a, FlowAttribute) else a

        nodes = {name: {k: get_val(v) for k, v in attr.items()} for name, attr in self.nodes.items()}
        state = {
            "name": self.name,
            "nodes": nodes,
            "links": self.links,
            "inputs": self.inputs,
            "outputs": self.outputs,
            "params": self.params,
            "hidden_inputs": self.hidden_inputs,
            "config": self.config,
            "post_config": self.post_config,
        }
        return state

(The post_config should not be added to the state)

For me it is yet unclear if this can be fixed without breaking things, by e.g. setting post_config to None. As no one ever detected this bug, we might assume no one ever tried to use the post_config.

@michelwi
Copy link
Contributor

As no one ever detected this bug, we might assume no one ever tried to use the post_config.

I would not say that. hash Bugs have very long been present in our setups and nobody noticed until we tried to move one setup and then found the hashes to be different..

unclear if this can be fixed without breaking things, by e.g. setting post_config to None.

I do not know any flow that uses a post config, but I do not know all flows by heart.
You can make a PR with this change and then we will notice if any hashes are broken on AppTeks site.

@vieting
Copy link
Contributor

vieting commented Jul 27, 2023

You can make a PR with this change and then we will notice if any hashes are broken on AppTeks site.

The AppTek hash test still passes, see #437.

@vieting
Copy link
Contributor

vieting commented Jul 27, 2023

Closed with #437.

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

3 participants