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

JBIDE-18267 - ensure agent does not attach to newly detected jvms until ... #309

Merged
merged 1 commit into from Dec 4, 2014

Conversation

@robstryker
Copy link
Member

robstryker commented Dec 2, 2014

...user connects to it in jmx view

@robstryker robstryker force-pushed the robstryker:JBIDE-18267 branch from df1bd72 to f9b2218 Dec 3, 2014
@mmalina
Copy link
Member

mmalina commented Dec 3, 2014

I checked this PR locally and it seems ok. The error with EAP 5.2 is gone and connection to both EAP 5.2 and EAP 6.x still works ok.

* @return The active JVM
* @throws JvmCoreException
*/
IActiveJvm addLocalActiveJvm(int pid, String mainClass,

This comment has been minimized.

Copy link
@maxandersen

maxandersen Dec 3, 2014

Member

is this supposed to be default scoped ? and does it not change api that fuse uses ?

This comment has been minimized.

Copy link
@maxandersen

maxandersen Dec 3, 2014

Member

never mind - pubic is default ;) ...anyway, same question applies - does this not change api?

This comment has been minimized.

Copy link
@robstryker

robstryker Dec 3, 2014

Author Member

The interface doesn't say it, but it's really not for external implementation (as far as I can tell). A grep on fuseide's source tree shows they import the interface, and use it, but no class in fuseide implements it.

Technically, yes, this is an API break. But I saw no other solution other than adding an interface method... I tried three other workarounds, but short of having jvmmonitor cast to internal IHost implementations (which is a bad idea), nothing else would have worked.

@maxandersen
Copy link
Member

maxandersen commented Dec 3, 2014

this seem to look like its changing public api in backwards incompatible way or am I reading this wrong ?

@maxandersen
Copy link
Member

maxandersen commented Dec 3, 2014

@mmalina can you reproduce the eap 5.2 error when this patch is not there ?

@robstryker
Copy link
Member Author

robstryker commented Dec 3, 2014

Martin indicated to me he was able to replicate it 5 minutes before installing the locally-built units.

mmalina: yes!
strykerawb: fixed?
mmalina: I'm not that far yet - I only just verified that I can see the original issue now :) now installing your PR

@mmalina
Copy link
Member

mmalina commented Dec 3, 2014

@maxandersen yes, I could reproduce the error before I applied this patch. And then when I installed it, the problem disappeared.

@robstryker robstryker force-pushed the robstryker:JBIDE-18267 branch 2 times, most recently from cb81e54 to 5a96d23 Dec 3, 2014
…il user connects to it in jmx view

JBIDE-18267 mark interface as not to be implemented by clients
@robstryker robstryker force-pushed the robstryker:JBIDE-18267 branch from 5a96d23 to 9decbc1 Dec 3, 2014
@robstryker robstryker merged commit 9decbc1 into jbosstools:master Dec 4, 2014
@robstryker robstryker deleted the robstryker:JBIDE-18267 branch Jan 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.