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

[MRESOLVER-266] Simplify named lock adapter creation and align config source #196

Merged
merged 1 commit into from
Sep 24, 2022

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Sep 16, 2022

Simplified adapter creation and now as everything else in resolver, session
config properties are used as configuration source, not Java system
properties.


Note: This PR contains commit d76008f that
drop unused 3 classes (unused due simplification). This may be considered as a breakage,
IF someone uses internal/private/impl
details from resolver (mvnd does, but is fine, is our kiddo),
and for the rest of the worlds: fingers off please.


https://issues.apache.org/jira/browse/MRESOLVER-266


Supersede #188 (same change but PR recreated)

@cstamas cstamas self-assigned this Sep 16, 2022
@cstamas cstamas requested review from michael-o and gnodet September 16, 2022 16:32
Copy link
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

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

Testing..

…uration

Simplified adapter creation and now as everything else in Resolver, session
config properties are used as configuration source, not Java system
properties.

Important: This change also drops internal/impl things. Therefore, this is a
breakage, IF someone uses internal/private/impl details from Resolver
(mvnd does, but is fine, is our kiddo), and for the rest of the worlds:
hands off, please.

This closes apache#196
Copy link
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

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

Rebased. squashed, cleaned up commit message. Really good to merge...

@cstamas cstamas merged commit 23197f4 into apache:master Sep 24, 2022
@cstamas cstamas deleted the MRESOLVER-266 branch September 24, 2022 21:29
cstamas added a commit to cstamas/maven-resolver that referenced this pull request Oct 27, 2022
Key changes:
* It is not session to register handlers against, but RepositorySystem
* DefaultRepositorySystemSession class deprecated, but will work as intended with a
  slight semantic change (it's handlers are executed on repo system shutdown)
* introduced MutableRepositorySystemSession and used instead, session creation
  should be done using RepositorySystem and handle it as a resource
* locking: partially undo PR apache#196, but do not use System properties but
  container injecter parameters instead (works with SISU only!)
cstamas added a commit to cstamas/maven-resolver that referenced this pull request Oct 27, 2022
Key changes:
* It is not session to register handlers against, but RepositorySystem
* DefaultRepositorySystemSession class deprecated, but will work as intended with a
  slight semantic change (it's handlers are executed on repo system shutdown)
* introduced MutableRepositorySystemSession and used instead, session creation
  should be done using RepositorySystem and handle it as a resource
* locking: partially undo PR apache#196, but do not use System properties but
  container injecter parameters instead (works with SISU only!)
cstamas added a commit to cstamas/maven-resolver that referenced this pull request Oct 28, 2022
Key changes:
* It is not session to register handlers against, but RepositorySystem
* DefaultRepositorySystemSession class deprecated, but will work as intended with a
  slight semantic change (it's handlers are executed on repo system shutdown)
* introduced MutableRepositorySystemSession and used instead, session creation
  should be done using RepositorySystem and handle it as a resource
* locking: partially undo PR apache#196, but do not use System properties but
  container injecter parameters instead (works with SISU only!)
cstamas added a commit to cstamas/maven-resolver that referenced this pull request Oct 28, 2022
Key changes:
* It is not session to register handlers against, but RepositorySystem
* DefaultRepositorySystemSession class deprecated, but will work as intended with a
  slight semantic change (it's handlers are executed on repo system shutdown)
* introduced MutableRepositorySystemSession and used instead, session creation
  should be done using RepositorySystem and handle it as a resource
* locking: partially undo PR apache#196, but do not use System properties but
  container injecter parameters instead (works with SISU only!)
cstamas added a commit to cstamas/maven-resolver that referenced this pull request Oct 28, 2022
Key changes:
* It is not session to register handlers against, but RepositorySystem
* DefaultRepositorySystemSession class deprecated, but will work as intended with a
  slight semantic change (it's handlers are executed on repo system shutdown)
* introduced MutableRepositorySystemSession and used instead, session creation
  should be done using RepositorySystem and handle it as a resource
* locking: partially undo PR apache#196, but do not use System properties but
  container injecter parameters instead (works with SISU only!)
cstamas added a commit that referenced this pull request Oct 28, 2022
Drops problematic onSessionClose, keeps only onSystemClose support. Issue will be adjusted accordingly.

Key changes:
* undone all related to onSessionClose
* kept onRepositorySystemClose + shutdown
* all 3 users on onSessionClose (locking + 2 "recording" features) will act on shutdown
* moved out of session.data use as it is completely unreliable, now singleton components hold caches (hence the shutdown persistence)
* DefaultSyncContext: fixed as explained, does not use system properties anymore, but is made pretty much same as before #196
* renamed all new components to `camelCase`

----

https://issues.apache.org/jira/browse/MRESOLVER-278
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.

3 participants