Closed Andrapyre closed 5 months ago
@xerial , before finalizing and adding tests, could I get an overall thumbs-up on this implementation? Would like to make sure that there's nothing controversial here, otherwise time spent on testing could be wasted effort.
Formatting issues are fixed - all workflows should now succeed. If there are no issues with the overall approach, I'll add some tests and finalize.
@Andrapyre First of all, thanks for preparing this PR. It gives us a head start. I also could learn a lot about the new Sonatype API.
sonatypeDeploymentName
(String). I believe there is no need to expose the internal DeploymentName model class to users. As central will eventually be the default repository, we can remove Central
from the key name. sonatypeRelease
(for automatic deployment) and sonatypeUpload
(for user-managerd deployment), so that sonatypeBundleRelease
can call sonatypeRelease internally and the user can choose what they need. Note: You don't need to tackle the items mentioned here, as I will be able to manage them.
After receiving your responses, I can suggest further adjustments to the PR, or I can make these changes myself to save your time, depending on your preference.
@xerial , thanks for the quick response. Several points:
The deployment name is purely cosmetic in Sonatype Central, so that the user can meaningfully distinguish in the ui between different deployments. Multiple deployments can even have the same name and if no name is provided then they will automatically be assigned the filename (in this case bundle.zip
) as the deployment name. To avoid the scenario where the user logs in and sees 100 "bundle.zip" deployments with no way to figure what is what, I've set the deployment name to "
Downstream plugins (such as typelevel sonatype and sbt-ci-release) use the sonatypeBundleRelease
command. To give users the freedom to specify whether they want their deployment to be managed or automatic without needing to make downsteam changes, I would introduce a new key, isSonatypeReleaseAutomatic
and have sonatypeBundleRelease
pass the value down accordingly, while keeping the rest of the functionality as is in this PR.
I will make a best effort attempt to migrate to airframe codec. It should be pretty easy. If it gets time-consuming or I run into larger issues, I will leave as-is or pick a different framework, if you prefer (jsoniter-scala is very fast/the fastest and well maintained - I've had a very good experience with it in other projects). I agree in general that fewer dependencies are better and would have preferred to introduce none with this feature. I chose circe in part because it has first-class support in sttp.
I appreciate that Scala can be difficult to read and work with, especially for new beginners. In my experience though, that generally applies to use of effects - I haven't seen anyone complain about simple types like Either or Option. Are you referring to my utils
package with the leftMap
implementation? I can add a few comments so that it's clear what those are practically doing. Otherwise, I find using Either to wrap exceptions far less messy than putting try/catch everywhere and would prefer not to change it if possible.
Let me know your thoughts. I'll proceed in any case with revising the json parsing away from circe.
One more thought - I wouldn't hold off on merging this until airframe provides multipart support. There are users that need this functionality ASAP (including me) and I'm not sure that multipart uploads, including support for octet streams, will be a quick and simple feature add. Could we merge first and then come back and refactor when airframe provides multipart support?
@Andrapyre The internal implementation can be changed later, so merging this first works. You need to address only two points addressed as major here https://github.com/xerial/sbt-sonatype/pull/474#issuecomment-1967331705
sonatypeRelease
~ sonatypeReleaseCentral (automated release at Central) internally sonatypeUpload
(for manual release)I'm against changing the behavior of sonatypeBundleRelease with an option (e.g., isSonatypeReleaseAutomatic ) because sonatypeBundleRelease
is supposed to complete all of the deployment steps at once. These command names and behavior will be difficult to change once the users start depending on the behavior.
See also existing commands for supporting such step-by-step executions https://github.com/xerial/sbt-sonatype?tab=readme-ov-file#individual-step-commands I believe not so many users are using these commands, though.
Understood - I'll make said changes and revert.
Alright. Several changes here:
sonatypeDeploymentName
setting is now a String type.zio-json
which has a similar structure to circe and has better support/maintenance long-term. I didn't see airframe codec or jsoniter-scala easily decoding the deployment stage enum we have, something that is very simple with both circe and zio-json.sonatypeCentralRelease
(automatic) and sonatypeCentralUpload
(user-managed) sonatypeBundleRelease
calls the same function as sonatypeCentralRelease
if the credential host is Sonatype Central. I will add some tests (mainly for the Sonatype Central client), along with documentation, and then request a final review.
Apologies for the delay. I'll be looking into this next week. And with Sonatype's recent announcement about account migration, it looks like I'll also need to get my repositories set up there.
All good - I had a delay myself due to other commitments taking priority. I did manage to test automatic deployments today, as well as a cross-versioned deployment to Sonatype Central, and everything worked as expected. I will forego tests for now for the sake of immediately merging this. The other client doesn't have testing as is and it seems that sttp might be ripped out in the near future and then all tests would need to be rewritten anyway. On that subject, if the plan is to keep sttp, I'm happy to add tests in a follow-up PR.
I still need to add some documentation and push all changes.
Documentation is added. Happy to receive a review now.
@xerial , I have moved all of the basic client operations into a client library here, where I will maintain and test it long term. I also updated some of the retry logic to immediately fail when it encounters user errors (e.g. the user specified wrong credentials, so we shouldn't retry for the next 30 minutes, etc.).
Could we get a 4.0.0-RC1 out in the next few days, so that the community can test this? Main question mark for me is how very large, cross-built projects with a lot of modules will be affected and whether we need to build out any extra support for such projects. With Sonatype Central, users can only upload one large bundle. It looks like the legacy oss environment supported parallel processing of some kind vis-a-vis this comment here? The same functionality isn't available in Central. Not sure about the potential breakage there.
@xerial Are there any blockers before this can be merged and an RC can be published? It would be amazing to be able to use sbt-sonatype
with Sonatype Central.
BTW, thanks for maintaining this library :heart:
@KacperFKorban , @xerial expressed concerns offline that there was too much overlap between legacy and newer functionality, when many of the features are quite different or mean totally different things. For instance, there are no staging repos, host names, snapshot repos, or profiles, etc. in the new publishing api. Nevertheless, working the new central portal api into this plugin requires us to set values for all or most of these, which makes the code harder to reason about and could lead to potential bugs down the road.
When I implemented this for mill
the team requested that this functionality be kept in a completely separate lib, though I also initially integrated it into their existing sonatype publishing lib. I've decided to take a similar route here and put this into a separate plugin, which I will maintain here. I'm having some issues deploying it - see https://github.com/lumidion/sbt-sonatype-central/issues/1. When these are resolved, I will deploy the plugin, close this pull request, and request that the corresponding issue be closed as well.
Hi folks. As I don't have much time to review or fix it, I'll ship it now and fix it (or review it) later.
Released 3.11.0 https://github.com/xerial/sbt-sonatype/releases/tag/v3.11.0
Description
This PR adds support for the new Sonatype Central publishing api, documented here and here. To use, ensure that your environment variables contain the appropriate credentials and add the following to your library
build.sbt
:New Dependencies
Adding these was necessary, since the airframe client does not support multipart uploads. Perhaps it is possible to custom-wire the airframe client so that it does this, but I was not successful in an initial try. There is, in any case, no airframe documentation about multipart uploads, nor could I find any support in the code base. The following dependencies made implementation very easy. I have added the "effect-free" versions, since the current
sbt-sonatype
codebase does not use any effects, though use of effects would make certain things like resource management easier.New SBT Keys
Users can customize Sonatype Central deployment names like so:
Additional Notes
This initial version hard-codes the bundle upload to a "user-managed" deployment - i.e. after the bundle is uploaded and verified, users will need to manually press "publish" in order for it to be published to Maven Central. This functionality is ideal for a release candidate version of
sbt-sonatype
, as it gives users the ability to verify that everything is working before publishing. A final version can either hard-code the deployment to "automatic" or introduce a new sbt key so that users can specify this themselves (preferred).Sonatype Central currently does not support publishing snapshots. The documentation says that an uploaded (but not published) deployment can be tested in production by referring to the deployment id, but I have not verified this.
I deployed a library to Sonatype Central using these new changes and it worked with no issues.