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 an API for releng to submit entirely new locales #7908

Closed
wagnerand opened this issue Aug 24, 2020 · 21 comments · Fixed by mozilla/addons-server#19342
Closed

Add an API for releng to submit entirely new locales #7908

wagnerand opened this issue Aug 24, 2020 · 21 comments · Fixed by mozilla/addons-server#19342
Assignees
Labels
component:api neverstale Use this to tell stalebot to not touch this issue. Should be used infrequently. repository:addons-server Issue relating to addons-server state:verified_fixed
Milestone

Comments

@wagnerand
Copy link
Member

The current signing API does not support submission of the first listed add-on version, due to required extra meta-data. To remove the obstacles and avoid breakage in the release process, we could add an API that support this specifically for langpack submissions from the releng account.

When a the first listed version gets submitted through the API, AMO will set the required metadata to some hard-coded defaults, for example setting the license etc.

/cc @escapewindow

@diox
Copy link
Member

diox commented Aug 31, 2020

I'd have to take a closer look at the existing signing API but I think it could be added there. See also #5348 which describes the missing data we'd need a default value for.

@eviljeff
Copy link
Member

eviljeff commented Sep 1, 2020

I still think like I said in #5348, we should take the opportunity to build basic support for creating via /api/addons (e.g. /api/addons/upload to handle the upload, then a POST|PUT on /api/addons to create the addon, with necessary metadata)

@diox
Copy link
Member

diox commented Sep 2, 2020

Note that currently, we don't automatically set target_locale for langpacks - this would need to be added as well, from the manifest langpack_id property (+ some conversion ? I don't remember)

@jvillalobos
Copy link

Creating a new API would be kind of defeating the purpose, no? It means that you need to know in advance that you need to do something different for new langpacks, and at that point you could as well just submit it manually. I think it makes sense to create a langpack listing if we get something we didn't have before via the current API. Langpack listings aren't particularly distinctive and they could be edited to something nicer later on if needed.

@escapewindow
Copy link

Releng submits langpacks automatically, and forcing a manual step in there breaks our automation.

@eviljeff eviljeff self-assigned this Dec 7, 2020
@eviljeff eviljeff modified the milestones: 2021.01.14, 2021.01.21 Jan 6, 2021
@eviljeff eviljeff removed this from the 2021.01.21 milestone Jan 19, 2021
@eviljeff eviljeff removed their assignment Feb 12, 2021
@stale
Copy link

stale bot commented Aug 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

@stale stale bot added the state:stale Issues marked as stale. These can be re-opened should there be plans to fix them. label Aug 18, 2021
@wagnerand wagnerand added neverstale Use this to tell stalebot to not touch this issue. Should be used infrequently. and removed state:stale Issues marked as stale. These can be re-opened should there be plans to fix them. labels Aug 18, 2021
@MihaiTabara
Copy link

This hit us in 91.0b2, it's still important for RelEng to get fixed on AMO side. Every time this breaks, it requires RelEng to perform the manual fix. It impacts RelMan as well if we're not around.

@MihaiTabara
Copy link

Note to self: TIL #5348 will be a good starting point to fixing this issue.

@wagnerand
Copy link
Member Author

(Please ignore my earlier comment in case you got an email notification).

I'd like to understand why you were hit by this recently. Until this API has been implemented, we have an established process to get new langpacks added manually. /cc @flodolo

Could you expand what went wrong or why that didn't work this time?

@escapewindow
Copy link

Per this issue's description, the API requested is to add new locales (i.e., new langpacks), so we don't have to submit them manually. Requiring manual steps in an otherwise automated release blocks releases on a handful of individuals. The manual steps are a considerable pain point.

@flodolo
Copy link

flodolo commented Sep 14, 2021

Could you expand what went wrong or why that didn't work this time?

While I don't have any visibility over what happens on the RelEng's side, I don't think the problem happened only this time, it happens every time there's new locale (which is not frequent, possibly making things even worse).

There are some manual steps I need to do once the langpack is available, but that's acceptable on my side, given everything is manual to get to that point.

@eviljeff
Copy link
Member

#5348 would be the base for any support for lang packs to be submitted entirely automatically, but we'll keep this issue open because langpacks will certainly need some special support adding - if not a separate endpoint. (In particular, you need the ability to submit an xpi without knowing if it's a new add-on or a new version, which is something that ends up being problematic for add-ons in general.)

@eviljeff
Copy link
Member

mozilla/addons-server#19342 will allow an upload to be sent as a PUT request to /api/v5/addons/addon/<addon-guid>/

Some things to note:

  • it's not just limited to langpacks or releng (but the existing restriction about who can submit langpacks is still in place) - any add-on can be submitted this way by any add-on developer.
  • there isn't any automatic metadata for langpacks as suggested in comment0, but categories and license can specified in the JSON postdata (and summary if there isn't a description in the manifest).
  • the guid must be defined in the manifest, and match the guid in the url
  • this is a v5+ api feature (along with the rest of the add-on submission api work) so it's still a little experimental/work-in-progress. We can help with some more detailed guidance on the move over from the old signing api once releng is ready to switch (and is comfortable with the experimental-ness)

@eviljeff
Copy link
Member

Note that currently, we don't automatically set target_locale for langpacks - this would need to be added as well, from the manifest langpack_id property (+ some conversion ? I don't remember)

I forgot to double check this! We have some automatic detection for dictionaries but only dictionaries. Logged #8802

@ioanarusiczki
Copy link

@eviljeff I'm stuck with a 400 response even if I add version as form-data. Am I doing something wrong?

version is required

@eviljeff
Copy link
Member

@ioanarusiczki it looks like you're sending for data that would be sent for a version create request, but PUT needs the addon create request data.

@ioanarusiczki
Copy link

@eviljeff If I take version out it would throw the same error -> that version is required

@eviljeff
Copy link
Member

yes, a version object with the necessary metadata as object keys (at least upload) is required to create an add-on. The request is an add-on create request as per https://addons-server.readthedocs.io/en/latest/topics/api/addons.html#create

@ioanarusiczki
Copy link

@eviljeff Ah, I thought the whole idea is to make one request and skip the upload , I got confused reading the discussions around it. Alright , then I know what to do, thanks!

@ioanarusiczki
Copy link

ioanarusiczki commented Jun 16, 2022

@eviljeff

I verified this on -dev:

  • using PUT I could create and upload new versions for all addon types.
  • I could not submit a lang pack without the lang pack permission.
  • filed Addons can be created without name/summary in the default-locale #8804 for an issue noticed.
  • the new version + update will make an update for the fields that could be changed with PATCH addons endpoint (and not what could be edited with PATCH versions) which I believe it is the expected.
  • when I create a new upload for the PUT request but the manifest has an id and version which was used to create before - I get at first an error such as ""Upload is not valid." . Then I repeat the PUT request (with the same upload as before) and the message changes to ""Version 1.45 already exists." - > perhaps it should throw only "Version 1.45 already exists." from the first attempt ?

@eviljeff
Copy link
Member

  • when I create a new upload for the PUT request but the manifest has an id and version which was used to create before - I get at first an error such as ""Upload is not valid." . Then I repeat the PUT request (with the same upload as before) and the message changes to ""Version 1.45 already exists." - > perhaps it should throw only "Version 1.45 already exists." from the first attempt ?

yeah, that's odd - it should be a consistent error. That it's changing implies that something is saved/changed even when there's an error, which is also isn't good. Can you file a follow-up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api neverstale Use this to tell stalebot to not touch this issue. Should be used infrequently. repository:addons-server Issue relating to addons-server state:verified_fixed
Development

Successfully merging a pull request may close this issue.

9 participants