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

add delta flag to asset id if needed #301

Merged

Conversation

asplinsol
Copy link
Contributor

This PR fixes an error I was seeing while publishing a release using electron-forge/publisher-eletron-release-server.

The assets being uploaded included both a full and delta nupkg and electron-release-server returned a HTTP error 400 with the error explanation that the asset id already exists.

The fix is to include delta in the generated asset ID so that it is a unique primary key.

@asplinsol
Copy link
Contributor Author

@ArekSredzki my PR I believe is for the same issue as PR #241 but handled in a much simpler way (in keeping with other parts of the code that check for a delta string in the name).

@lauer
Copy link
Contributor

lauer commented Oct 6, 2022

Thanks for your fix, I was testing the same plugin (publisher-electron-release-server), but it stopped me because if this error. So i still upload using SSH to a server.

@ArekSredzki Could you please review and merge this change?

@asplinsol are you using this release-server in prod somewhere? and on an open-source project where you share the implementation?

@jasplin
Copy link

jasplin commented Oct 6, 2022

@lauer thanks for the comments, you may want to also look at #302 in case you need it?

At the moment I've put my use of this release-server on hold - I had more errors and ran out of time on the project. Like you I'm still using alternatives.

I might come back to it when I have some time because I really like the customer friendly, flexible UI but until then I will be monitoring for others to contribute :)

@lauer
Copy link
Contributor

lauer commented Oct 7, 2022

@jasplin What errors did you also have? And which solution do you have now?
I was hoping that all the PR's you created combined to make the app stable enough to be used for a small project I have.

@ArekSredzki ArekSredzki merged commit be26fb4 into ArekSredzki:master Dec 20, 2022
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.

4 participants