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

Throw error if component has __post_configure__ but is not configured #280

Open
jneeven opened this issue Sep 22, 2021 · 3 comments
Open

Comments

@jneeven
Copy link
Contributor

jneeven commented Sep 22, 2021

Feature motivation

It's currently possible to define a component with some crucial asserts in the __post_configure__, and then use it without configuring it, hence never triggering those asserts.

Feature description

It would be nice to check if a component has a custom __post_configure__, but I'm not sure where the best place to do that would be. It probably gets tricky quite quickly and may need to be done in e.g. __getattribute__.

@AdamHillier
Copy link
Contributor

It probably gets tricky quite quickly and may need to be done in e.g. __getattribute__.

I definitely agree with the motivation but I think doing this might be tricky, because in some scenarios we want __getattribute__ to work prior to configuration (e.g. in __pre_configure__) and I'm not sure how we'd tell the difference.

@AdamHillier
Copy link
Contributor

We definitely could do a check specifically for @tasks prior to calling the run method. Would that be useful do you think?

@jneeven
Copy link
Contributor Author

jneeven commented Sep 22, 2021

We definitely could do a check specifically for @tasks prior to calling the run method. Would that be useful do you think?

I ran into it with a component, so that probably only strengthens the incorrect assumptions that it'd get checked even without configuring it... I'll have a think about possible ways to do this

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

Successfully merging a pull request may close this issue.

2 participants