Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Stéphane Épardaud
    @FroMage
    it's not been updated in the marketplace for the latest Eclipse release (from early this year)
    and the manual install site is 404
    @njr-11 IIRC we moved to testng to get in line with other MP specs, right?
    frankly that surprised me, because I thought testng died a decade ago, but I didn't really have much to stand on, besides "junit is moving a lot more today"
    but this lack of IDE support is really problematic
    if you would not mind going back to junit, I can send a mail to the MP mailing list discussing a move to junit
    Stéphane Épardaud
    @FroMage
    ok, my bad, the manual install site is working now, it probably was down when I hit it
    it's still pretty annoying to use testng, though :(
    Nathan Rauh
    @njr-11
    @FroMage sorry I was out of the office for a week and wasn't able to keep up on email/chats. If I understand correctly, it sounds like the immediate issue with testng was resolved. As to whether we can switch back to junit, I think that's a question of alignment with the other MicroProfile specifications. If all of the others are using testng, then we need to do the same for consistency, but if the others are varied between testng/junit, then we should be fine switching.
    Stéphane Épardaud
    @FroMage
    Yeah, I wasn't suggesting switching alone, but raising the question for all MP specs, but only if you don't mind, otherwise there's not much point :)
    Nathan Rauh
    @njr-11
    That sounds good - raise the question for MP as a whole. I don't have any objections to switching
    Stéphane Épardaud
    @FroMage
    ok thanks
    Nathan Rauh
    @njr-11
    I've added #174 to update our version number to 1.1 so that it will be clear that new snapshot builds correspond to post-1.0 release. Could someone review it?
    Stéphane Épardaud
    @FroMage
    Done
    Matej Novotny
    @manovotn

    So, while trying to run the TCK with Quarkus I came across this one bit in tests - https://github.com/eclipse/microprofile-context-propagation/blob/master/tck/src/main/java/org/eclipse/microprofile/context/tck/ThreadContextTest.java#L155-L160

    This gives me crashes with IllegalAccessException probably because of class loader black magic that quarkus uses to execute Arquillian.
    Nonetheless, I find this approach very awkward at best, does anyone happen to remember why it's there?
    We could simply call (for instance) CDI.current().select(UserTransaction.class).isResolvable()

    @njr-11 @FroMage @mmusgrov ^^
    Stéphane Épardaud
    @FroMage
    well, that would make it depend on CDI, but apparently it already does, though it's not used
    it was added in fbe0b27eda0a14d54053352dc053f986272c8917
    for config, which was later removed but not the dependency
    so the pom depends on cdi but not the code
    a bit unfortunate
    Nathan Rauh
    @njr-11
    I believe someone raised a requirement at one point for MP Context Propagation to be usable regardless of whether CDI was available. So even back when we had the config annotations (@ManagedExecutorConfig) the TCK tests went out of their way to ensure that an implementation could pass when CDI was not available.
    Looking ahead a couple of lines in the test, that test isn't really concerned with CDI at all, and just wants a UserTransaction. It tried two different ways to get one, requiring at least one to succeed, and then using whichever that is. It even traps for errors, although didn't expect an IllegalAccessException as a type of error to trap for. Does Quarkus allow the other mechanism of obtaining UserTransaction? If so, this might be as simple as adding a catch block to the test
    Matej Novotny
    @manovotn
    I think my question was more along the lines of - do we keep the claim that its CDI independent? We even have TCKs taht directly operate on CDI beans anyway, so its not like the dependency isn't there already
    and to answer your question, InitialContext.doLookup() wont work as we dont have EJBs there
    and this other approach I am not quite sure yet why it fails, but I suppose it's because of class loader shuffling
    Matej Novotny
    @manovotn
    also if you insist in it passing without CDI impl, then we could make a check if CDI is present via just API using CDI.current() (throws IllegalStateException is no provider is found) and if that's ok, then proceed using Instance.select() just without the reflections since we depend in the API anyway?
    Nathan Rauh
    @njr-11
    To clarify, I wasn't the one insisting upon the TCK being able to pass without a CDI implementation, nor did the request come from any of the spec participants who were involved in the OpenLiberty implementation. If I had to guess prior to this conversation, I would have guessed that Stephane had raised the requirement at one point, but it must have been someone else. If we're going to change it, let's get an issue open because that will have broader visibility and hopefully allow for any objections to be surfaced.
    Matej Novotny
    @manovotn
    poor wording from me, didn't mean to "blame it on you", sorry for that :)
    Sure, I'd open an issue to at least make it the way I suggested above, to check via API for actual impl and skip the tests if not present, otherwise proceed with CDI in place
    Matej Novotny
    @manovotn
    Matej Novotny
    @manovotn
    Stéphane Épardaud
    @FroMage
    It was totally me asking to not depend on CDI
    At the time we wanted to use this in RESTEasy which does not depend on CDI
    Now that the next version of JAX-RS is going to require CDI, and Quarkus requires CDI, it's become moot
    And besides, the unused build dep slipped through
    Now, I mostly wanted the spec to not require CDI, and I guess for the TCK to not require that implementations of the spec depend on CDI. As for the TCK itself depending on CDI I don't think it would have mattered
    Matej Novotny
    @manovotn
    @njr-11 @FroMage with the recent TCK changes in, would you guys be ok with a new MP CP release? The set should be complete to allow us to run the TCKs on quarkus (with one exclusion that is to-be-implemented soon)
    since there would be no new features, we could do it as some sort of "SP" release instead of bumping the minor version if that's more desirable...
    Stéphane Épardaud
    @FroMage
    Fine by me, we can make a new minor release.
    Nathan Rauh
    @njr-11
    I'll ask around about what the right way to do this is: a point release or a SP release. So far, it's only TCK updates, and no actual changes to API
    Stéphane Épardaud
    @FroMage
    Well we did remove a CDI (unused) dependency
    1.1 was only changes in the javadoc IIRC
    Nathan Rauh
    @njr-11
    Good point, #174, which is the update from 1.0 to 1.1 would need to be reverted in order to make a micro release off of 1.0 for this. That doesn't look too difficult, and the same changes could be reapplied afterward
    Nathan Rauh
    @njr-11
    The revert was merged under #181 and I have started the process of creating a 1.0.1 release with the TCK updates
    I've also created #182 to restore us back to 1.1 once all of this is finished.
    Matej Novotny
    @manovotn
    @njr-11 awesome, thanks!
    Nathan Rauh
    @njr-11
    The 1.0.1 version should be accessible in Maven now for testing, although it will likely take a few days to show up here:
    https://mvnrepository.com/artifact/org.eclipse.microprofile.context-propagation/microprofile-context-propagation-tck
    It still runs cleanly on OpenLiberty. I'll wait for confirmation regarding Quarkus before merging #182 to switch us back to 1.1
    Matej Novotny
    @manovotn
    Noticed, already updated both, smallrye and quarkus with those versions ;)
    thanks a lot for handling the release
    Nathan Rauh
    @njr-11
    The following was sent by Ken Finnegan to the general MicroProfile mailing list:
    "For those that may not have seen, we've decided to update to Jakarta EE 8 APIs for the MP 3.3 platform release in Feb 2020.
    As a consequence of that decision, we need every specification to provide a minor update that includes the new Jakarta EE 8 APIs. There's no requirement that a specification provide any feature updates, but there must be a release that includes the new APIs so that we can align for MP 3.3
    I don't know the exact date as to when specifications will need to be released for MP 3.3, John took an action to update the calendar in today's MP Architecture call, but it's probably around mid-January.
    I would appreciate everyone's cooperation in getting a release of each specification by mid-January to ensure MP 3.3 is a smooth transition to Jakarta EE APIs."
    While MP Context Propagation is still an independent release and isn't yet in the platform, it seems like it would still make sense to align with the MicroProfile platform around support for the Jakarta EE 8 spec. As far as I'm aware, however, the only updates we have would be to dependencies (CDI) for building the TCK against Jakarta rather than Java EE.
    Matej Novotny
    @manovotn