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

Self heal #116

Closed
wants to merge 2 commits into from
Closed

Self heal #116

wants to merge 2 commits into from

Conversation

vijairaj
Copy link
Contributor

Fixes #114

@kviktor
Copy link

kviktor commented Jan 5, 2018

Hey, with this patch I have full files however for some reason import doesn't work on the first try only after that (it generates the source successfully into the cache dir but can't import them only after the fist run)

(I know this is almost a year old issue and maybe it is not even related to it ...)

@vijairaj
Copy link
Contributor Author

vijairaj commented Jan 7, 2018

@kviktor, is the issue consistent? Can you please add more details describing the failure seen during the first time import.

@junkmd
Copy link
Collaborator

junkmd commented Jun 19, 2022

#300 prevents partial files from being generated.

Processing order has changed.

previously: open file -> write header -> parse type library -> generate code -> write body and footer -> close file
currently: parse type library -> generate code including header and footer -> open file -> write code -> close file

The interval between opening a file, writing code, and closing it is much shorter, so "an interruption in between" is much rarer.

@vijairaj
Copy link
Contributor Author

vijairaj commented Jul 3, 2022

@junkmd -- While the changed order definitely makes the issue very rare but the self heal mechanism can act as 2nd level of protection against the small odds or when the file is corrupted later.

@junkmd
Copy link
Collaborator

junkmd commented Jul 3, 2022

@vijairaj
Certainly, importing from an empty or partial .py file can still occur in this situation, so I agree with the inclusion of self heal logic.
However, a long time has passed since this PR was first posted, the syntax has become Python 3 compliant, and the design of the codegenerator has changed.
If we were to reintroduce it now, what would be the best way to implement it?

junkmd added a commit to junkmd/comtypes that referenced this pull request Jul 30, 2022
junkmd added a commit to junkmd/comtypes that referenced this pull request Jul 30, 2022
junkmd added a commit to junkmd/comtypes that referenced this pull request Jul 30, 2022
…s in tools.codegenerator and client._generate. (related to enthought#114 and enthought#116)
@junkmd
Copy link
Collaborator

junkmd commented Jul 30, 2022

@vijairaj

While I am waiting for a review of the PR I have currently submitted, I thought about what would happen if I applied your proposed logic to the current master branch.

This code worked at runtime, but there are still problems.
https://github.com/enthought/comtypes/compare/944e279..2669789

  • The code calling another module's _comtypes_validate_file is written in a strange position.

  • To solve the import failing problem in my env(is related to mentioned by @kviktor(Self heal #116 (comment))?), instead of calling _comtypes_validate_file in _my_import, calling _comtypes_validate_file within trying to create or import the wrapper module.

  • _comtypes_validate_file definition is not PEP8-compliant.

  • Tests have not yet been created. I haven't figured out how to prepare the "partial module file" and how to see repaired it.

What is your opinion?

@junkmd
Copy link
Collaborator

junkmd commented Dec 8, 2022

@vijairaj
@kviktor

As mentioned in #114, good suggestion about the ability to restore partial modules.

However, it is difficult to implement and also difficult to validate with unittest.
As for testing, if only Python3 is supported, there is unittest.mock, so it could be done by patching sys.modules and import-related processes.

And the current code and module generation process design has changed significantly since this PR was submitted, causing conflicts with the proposed code.

Due to no recent activity, I close this.
If you wish to add this feature, please rebase, resolve conflicts and re-open or submit a new PR.

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 this pull request may close these issues.

Partial wrapper files generated in gen folder
3 participants