Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Matej Novotny
    @manovotn
    @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
    Matej Novotny
    @manovotn
    Nathan Rauh
    @njr-11
    Excellent - Thanks. I reviewed and approved it.
    Nathan Rauh
    @njr-11
    Given that the only change to align with Jakarta EE 8 is the TCK dependency update (and I noticed #172 requiring us to exclude the EL dependency, which I opened a pull for), I've added a comment to #183 suggesting that this could just be another micro release (1.0.2) rather than a new minor version (1.1). We haven't merged any commits for new API since releasing 1.0, and staying at 1.0 would make it clear that nothing has changed.
    Matej Novotny
    @manovotn
    Fine by me, but as I commented on the issue, if other MP spec go with minor version bump, we might as well do that too.
    No harm done for us either way I think.
    Emily Jiang
    @Emily-Jiang
    Doing a micro release is ok @njr-11 as Context Propagation won't be included in umbrella release. If it does, you have to a minor release as micro releases will not be picked up by umbrella releases.
    Nathan Rauh
    @njr-11
    A 1.0.2 micro release for updating the TCK dependency to Jakarta EE is available here:
    https://mvnrepository.com/artifact/org.eclipse.microprofile.context-propagation/microprofile-context-propagation-api/1.0.2
    Matej Novotny
    @manovotn
    @FroMage @njr-11 Does any of you remember how we decided to handle SECURITY context propagation? We declare its propagation in the spec, but we only vaguely declare what it is and have no TCKs for it. There isn't a unified way to do security in MP, so we cannot easily test it (granted, JTA isn't part of MP either), unless we'd add an SPI for that (not saying we should).
    But with this in mind, we should maybe mention this in the spec text somehow, e.g. the reasons why there are no TCKs and that it's probably as optional as transaction propagation is.
    Nathan Rauh
    @njr-11
    I think the idea behind SECURITY context was basically Java EE - compatibility with Java (now Jakarta) EE Concurrency, which included security as one of the core context types. Security context doc was written very generically and sparsely in MicroProfile to avoid requiring any sort of dependency on Java EE and avoid tying implementations to Java EE, but still aligning with it and remaining compatible with it. Initially, TRANSACTION context was treated similarly, although we ended up elaborating more and even adding tests for it, with various checks in place allowing implementations to optionally include none/some/all of it. We could certainly add some Java/Jakarta EE related TCK tests for security context, and make them optional in the sense that tests pass if you don't have a concept of security context, although I suspect it would require the additional complexity of the developer who is running the tests needing to follow prerequisite vendor-specific instructions for defining users and such.
    Matej Novotny
    @manovotn
    Yeah, I gave it some more thought and I am not sure a TCK would make much sense given that MP has no specification for security and even in EE the solutions very between application servers.
    Emily Jiang
    @Emily-Jiang
    We are looking at integrating Context Propagation with Fault Tolerance, we have a question on CDI context, application context and transaction context.
    1. Does weld support session and conversation context propation?
    2. What does application context mean? Does it mean jndi context?
    3. When I specify transaction context propagation, will the method failure cause transaction rollback? What exactly does it mean?
    Ken Finnigan
    @kenfinnigan
    How can it be integrated with Context Prop outside the platform?
    Emily Jiang
    @Emily-Jiang
    We are in the planning stage. Making it optional is one of the choices. I planned to discuss this in MP call.
    Nathan Rauh
    @njr-11
    Application context includes the java:comp, java:module, and so forth JNDI namespace of the application component, as well as its thread context class loader..
    Transaction context propagation means that the task or completion stage runs under the same transaction of the thread that submits the task or creates the stage. If the task or completion stage wants to roll back the transaction, it can do so with the usual rollback API, or it can mark the transaction for rollback with setRollbackOnly. The spec does not have anything where we auto-rollback due to a failure, which would limit flexibility of implementation by forcing rollback on exception paths.
    Transaction context propagation also covers propagation of the lack of any transaction to a thread. So if the original thread was not running in a transaction to start with, then the task or completion stage will also run without any active transaction on the thread.
    Emily Jiang
    @Emily-Jiang
    awesome thanks @njr-11 !