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

Support for deferred proxies using JImplements #656

Closed
wants to merge 1 commit into from

Conversation

Thrameos
Copy link
Contributor

In tonight's endless PR, we change the pattern for proxy creation so that classes can implement interfaces before the JVM is actually started. Proxies created in this fashion are not actually checked until the first object using the Proxy is created, but are otherwise the same as normal proxies.

Fixes #653

@codecov-io
Copy link

Codecov Report

Merging #656 into master will decrease coverage by 0.13%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #656      +/-   ##
==========================================
- Coverage   78.52%   78.39%   -0.14%     
==========================================
  Files         118      118              
  Lines        8755     8774      +19     
  Branches     3715     3715              
==========================================
+ Hits         6875     6878       +3     
- Misses       1373     1389      +16     
  Partials      507      507              
Impacted Files Coverage Δ
jpype/_jproxy.py 77.52% <60.00%> (-16.76%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db42f22...90d8c12. Read the comment docs.

@Thrameos Thrameos added the enhancement Improvement in capability planned for future release label Mar 29, 2020

# Return the augmented class
return type("proxy.%s" % cls.__name__, (cls, _jpype._JProxy), members)
if _jpype.isStarted():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the conversation at #504 I'm not sure JVM is started is the right test for deciding whether we should be doing interface/override checking. I rather think explicit control is a good thing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do interface and override checking regardless. If it is prior to starting then checking is done at the first create, else we execute it immediately.

return actualIntf

def new(tp, *args, **kwargs):
actualIntf = init(cls)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to only do this once, as we do with the non deferred case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe line 49 ensures we do this only once.

@pelson
Copy link
Contributor

pelson commented Mar 29, 2020

I also took a shot at this in #659.

@Thrameos Thrameos closed this Apr 1, 2020
@Thrameos Thrameos deleted the deferred_proxy branch April 1, 2020 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement in capability planned for future release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JImplements requires JVM to be running
3 participants