The default bug view has changed. See this FAQ.

Funsize returns errors out and returns an HTTP 500 when requested for a non-existent partial

RESOLVED FIXED

Status

Release Engineering
Tools
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ffledgling, Assigned: Harsimran Singh, Mentored)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

3 years ago
Ideally when a client requests and invalid/non-existant partial from funsize, it should receive an HTTP 400 response code.

Currently, because of change of behavior in the `lookup` function in https://github.com/ffledgling/Senbonzakura/blob/master/senbonzakura/database/db.py#L67 the error handling used in https://github.com/ffledgling/Senbonzakura/blob/master/senbonzakura/frontend/api.py#L158-L165 is no longer correct.
We can no longer rely on DBError to alert us in case of a lookup failure (it's not really a "failure") we thus need to check for an empty return value in partial instead and adjust the logging accordingly as well.

The behaviour change was introduced in
https://github.com/ffledgling/Senbonzakura/commit/b606ca1dceed9d5d4200f061e0eef5e2ef1232d0
(Reporter)

Updated

3 years ago
Whiteboard: [good first bug]
(Assignee)

Comment 1

3 years ago
i went through the code and figure out since we return None which is not capture by except block in https://github.com/ffledgling/Senbonzakura/blob/master/senbonzakura/frontend/api.py#L158-L165 so our method is not fully reliable. 

What i think the best way would be is that the first thing we do is check for the partial being empty and generate an appropriate flask object and logging accordingly and return this response. 

Is this approach fine or better ones exists ?
(Reporter)

Comment 2

3 years ago
(In reply to harsimran from comment #1)
> i went through the code and figure out since we return None which is not
> capture by except block in
> https://github.com/ffledgling/Senbonzakura/blob/master/senbonzakura/frontend/
> api.py#L158-L165 so our method is not fully reliable. 
> 
> What i think the best way would be is that the first thing we do is check
> for the partial being empty and generate an appropriate flask object and
> logging accordingly and return this response. 
> 
> Is this approach fine or better ones exists ?

Sounds about right. Throw in a patch for feedback and I'll be able to tell you more!
(Assignee)

Comment 3

3 years ago
i was trying to fix this bug but i cannot fully understand what this line
 
https://github.com/ffledgling/Senbonzakura/blob/master/senbonzakura/database/db.py#L75

of code does and more specifically when will it return None.

Surprisingly Funsize started to generate the partial mar even though i returned the flask object with status code 400.
(celery log: http://pastebin.com/keEAt4nQ)

diff (for changes i have made ): http://pastebin.com/yGbP5Q6a

therefore i think this bug is deeply connect with line mention up above.
(Reporter)

Comment 4

3 years ago
(In reply to harsimran from comment #3)
> i was trying to fix this bug but i cannot fully understand what this line
>  
> https://github.com/ffledgling/Senbonzakura/blob/master/senbonzakura/database/
> db.py#L75
> 
> of code does and more specifically when will it return None.

This line simply looks up an entry with the given identifier in the database, if it exists it will return the database record corresponding to that identifier, if it doesn't the value of `partial` will be None.

> 
> Surprisingly Funsize started to generate the partial mar even though i
> returned the flask object with status code 400.
> (celery log: http://pastebin.com/keEAt4nQ)
> 
> diff (for changes i have made ): http://pastebin.com/yGbP5Q6a
> 
> therefore i think this bug is deeply connect with line mention up above.

I'm not sure what commands you ran here, but the triggering of a partial and actually getting the partial are two separate things.
If you trigger a partial from firefox 28 to firefox 29 (which is what all of the `./integration-test.sh` and `./curl-test trigger-release` do) then that's not where the 400 is supposed to be returned.

If you try to access a *non-existant* partial, i.e one you haven't triggered yet, then the Flask server will give you an HTTP 500 response code.
You can test this out by starting up the application.
Then from a separate tab/terminal you can run `curl -I localhost:5000/partial/identifierdoesnotexist`
Or alternatively you can access the same URL via the browser and see the HTTP 500.

If you still have doubts or questions, drop by IRC and we can have a chat.
(Assignee)

Comment 5

3 years ago
Created attachment 8468617 [details] [diff] [review]
0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch

please provide feedback on this patch
Attachment #8468617 - Flags: feedback?(ffledgling)
(Reporter)

Updated

3 years ago
Assignee: nobody → harsimran_hs4
Status: NEW → ASSIGNED
(Reporter)

Comment 6

3 years ago
Comment on attachment 8468617 [details] [diff] [review]
0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch

Review of attachment 8468617 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good overall, a couple of nits need to be fixed.

::: senbonzakura/frontend/api.py
@@ +164,2 @@
>          logging.warning('Record lookup for identifier %s failed' % identifier)
>          resp = flask.Response("{'result':'partial does not exist'}", status=404)

Let's get rid of this JSON response and simply return a 500 here.

@@ +164,4 @@
>          logging.warning('Record lookup for identifier %s failed' % identifier)
>          resp = flask.Response("{'result':'partial does not exist'}", status=404)
>      else:
> +        #handle the None case

A better description along the lines of "In case no record was found" or something would be great.

@@ +168,5 @@
> +        if partial is None:
> +            logging.info('Partial doesnot exists')
> +            resp = flask.Response("{result: '%s'}" %
> +                        "Partial doesnot exists",
> +                         status=400)

Replace 'Partial doesnot exists' with 'Partial with <identifier> not found. You need to trigger the partial before downloading it.'

Also replace 'Partial doesnot exists' in the log message with 'Request for invalid partial'
Attachment #8468617 - Flags: feedback?(ffledgling) → feedback+
(Assignee)

Comment 7

3 years ago
(In reply to Anhad Jai Singh (:ffledgling) from comment #6)
> Comment on attachment 8468617 [details] [diff] [review]
> 0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch
> 
> Review of attachment 8468617 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good overall, a couple of nits need to be fixed.
> 
> ::: senbonzakura/frontend/api.py
> @@ +164,2 @@
> >          logging.warning('Record lookup for identifier %s failed' % identifier)
> >          resp = flask.Response("{'result':'partial does not exist'}", status=404)
> 
> Let's get rid of this JSON response and simply return a 500 here.

why it`s better to get rid of JSON response and how to simply return 500 ? errorhandler or abort or something else ?

> 
> @@ +164,4 @@
> >          logging.warning('Record lookup for identifier %s failed' % identifier)
> >          resp = flask.Response("{'result':'partial does not exist'}", status=404)
> >      else:
> > +        #handle the None case
> 
> A better description along the lines of "In case no record was found" or
> something would be great.
> 
> @@ +168,5 @@
> > +        if partial is None:
> > +            logging.info('Partial doesnot exists')
> > +            resp = flask.Response("{result: '%s'}" %
> > +                        "Partial doesnot exists",
> > +                         status=400)
> 
> Replace 'Partial doesnot exists' with 'Partial with <identifier> not found.
> You need to trigger the partial before downloading it.'

Didn`t get this line : trigger the partial before download 

> 
> Also replace 'Partial doesnot exists' in the log message with 'Request for
> invalid partial'
(Reporter)

Comment 8

3 years ago
(In reply to Harsimran Singh from comment #7)
> (In reply to Anhad Jai Singh (:ffledgling) from comment #6)
> > Comment on attachment 8468617 [details] [diff] [review]
> > 0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch
> > 
> > Review of attachment 8468617 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Looks good overall, a couple of nits need to be fixed.
> > 
> > ::: senbonzakura/frontend/api.py
> > @@ +164,2 @@
> > >          logging.warning('Record lookup for identifier %s failed' % identifier)
> > >          resp = flask.Response("{'result':'partial does not exist'}", status=404)
> > 
> > Let's get rid of this JSON response and simply return a 500 here.
> 
> why it`s better to get rid of JSON response and how to simply return 500 ?
> errorhandler or abort or something else ?
> 

An abort() call should be fine, we don't want to send JSON saying '{"result": "whatever" }' because there is technically speaking no result, the server simply encountered an error while processing the request.

> > 
> > @@ +164,4 @@
> > >          logging.warning('Record lookup for identifier %s failed' % identifier)
> > >          resp = flask.Response("{'result':'partial does not exist'}", status=404)
> > >      else:
> > > +        #handle the None case
> > 
> > A better description along the lines of "In case no record was found" or
> > something would be great.
> > 
> > @@ +168,5 @@
> > > +        if partial is None:
> > > +            logging.info('Partial doesnot exists')
> > > +            resp = flask.Response("{result: '%s'}" %
> > > +                        "Partial doesnot exists",
> > > +                         status=400)
> > 
> > Replace 'Partial doesnot exists' with 'Partial with <identifier> not found.
> > You need to trigger the partial before downloading it.'
> 
> Didn`t get this line : trigger the partial before download 
> 

If you read through https://wiki.mozilla.org/ReleaseEngineering/Funsize/API , you'll realize that there are two things that need to be done:
i) Tell the application to generate or create the partial we want (aka Trigger the partial generation first.)
ii) Actually download that partial once it's generated.

(i) can be done by sending a POST request to <app url>/partial with the correct parameters.
(ii) can be done by sending a GET request to <app url>/partial/<partial's identifier>

> > 
> > Also replace 'Partial doesnot exists' in the log message with 'Request for
> > invalid partial'
(Assignee)

Comment 9

3 years ago
Created attachment 8470147 [details] [diff] [review]
0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch

Please review the patch and suggest changes if any.
Attachment #8468617 - Attachment is obsolete: true
Attachment #8470147 - Flags: review?(ffledgling)
(Reporter)

Comment 10

3 years ago
Comment on attachment 8470147 [details] [diff] [review]
0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch

Review of attachment 8470147 [details] [diff] [review]:
-----------------------------------------------------------------

This patch looks good for the most part.
Giving this patch an r-, because there's a couple of nits that need to be fixed that I've pointed out inline.

Make sure to run all the integration-test and the curl-test to make sure we don't break something else accidentally.
If you have your own github that you work on, you can use the .travs.yml present in the repository to run the tests automatically on every push.
(This will help catch errors and help us give us more confidence that we don' break things.


Good work with this one.
Make the needed changes and re-submit for review, and hopefully we'll be able to check this in!

::: senbonzakura/frontend/api.py
@@ +166,2 @@
>      else:
> +        #No record found with this identifier

Space between '#' and 'No' keeping with the style throughout the rest of the code.

@@ +167,5 @@
> +        #No record found with this identifier
> +        if partial is None:
> +            logging.info('Request for invalid partial')
> +            resp = flask.Response("{result: '%s'}" %
> +                        "Partial with %s not found" % identifier,

Should be "Partial with identifier %s not found".
Attachment #8470147 - Flags: review?(ffledgling) → review-
(Assignee)

Comment 11

3 years ago
(In reply to Anhad Jai Singh (:ffledgling) from comment #10)
> Comment on attachment 8470147 [details] [diff] [review]
> 0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch
> 
> Review of attachment 8470147 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This patch looks good for the most part.
> Giving this patch an r-, because there's a couple of nits that need to be
> fixed that I've pointed out inline.
> 
> Make sure to run all the integration-test and the curl-test to make sure we
> don't break something else accidentally.

i ran integration test and the test failed. here is the output http://pastebin.com/d3Nm7QjJ for your reference. i hope this is the way integration tests need to be run.

> If you have your own github that you work on, you can use the .travs.yml
> present in the repository to run the tests automatically on every push.
> (This will help catch errors and help us give us more confidence that we
> don' break things.
> 

i have no idea how to use this ?

> 
> Good work with this one.
> Make the needed changes and re-submit for review, and hopefully we'll be
> able to check this in!
> 
> ::: senbonzakura/frontend/api.py
> @@ +166,2 @@
> >      else:
> > +        #No record found with this identifier
> 
> Space between '#' and 'No' keeping with the style throughout the rest of the
> code.
> 
> @@ +167,5 @@
> > +        #No record found with this identifier
> > +        if partial is None:
> > +            logging.info('Request for invalid partial')
> > +            resp = flask.Response("{result: '%s'}" %
> > +                        "Partial with %s not found" % identifier,
> 
> Should be "Partial with identifier %s not found".
(Reporter)

Comment 12

3 years ago
(In reply to Harsimran Singh from comment #11)
> (In reply to Anhad Jai Singh (:ffledgling) from comment #10)
> > Comment on attachment 8470147 [details] [diff] [review]
> > 0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch
> > 
> > Review of attachment 8470147 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > This patch looks good for the most part.
> > Giving this patch an r-, because there's a couple of nits that need to be
> > fixed that I've pointed out inline.
> > 
> > Make sure to run all the integration-test and the curl-test to make sure we
> > don't break something else accidentally.
> 
> i ran integration test and the test failed. here is the output
> http://pastebin.com/d3Nm7QjJ for your reference. i hope this is the way
> integration tests need to be run.

The test looks like it ran correctly for the most part, but there is a known issue with hashes on different operating systems, try passing a `-t` flag to the script and see if the test passes.
If not, let me know and we can file a bug.

> > If you have your own github that you work on, you can use the .travs.yml
> > present in the repository to run the tests automatically on every push.
> > (This will help catch errors and help us give us more confidence that we
> > don' break things.
> > 
> 
> i have no idea how to use this ?
You'll need a couple to do a couple of things to get this setup, you can skip it if you want to, but I'll list out the steps incase you're interested.
(You can work with this next bug onwards in-case you're interested or not use it at all if you prefer that)

1. You'll need a github.com account.
2. You'll need to fork the build-funsize repository to your own account
3. You'll need to clone your fork of the github repo to your local machine
4. Go to travis.org, and follow steps #1 and #2here -- http://docs.travis-ci.com/user/getting-started/
5. Make changes to your branch, once you're done, push your changes to github.
6. You should be able to see your test running at travis-ci.org if you did everything correctly.

If you need more info regarding travis-ci , see http://docs.travis-ci.com/ and look for the relevant section.

As always if you have any questions or need any clarifications feel free to ask in IRC.

> 
> > 
> > Good work with this one.
> > Make the needed changes and re-submit for review, and hopefully we'll be
> > able to check this in!
> > 
> > ::: senbonzakura/frontend/api.py
> > @@ +166,2 @@
> > >      else:
> > > +        #No record found with this identifier
> > 
> > Space between '#' and 'No' keeping with the style throughout the rest of the
> > code.
> > 
> > @@ +167,5 @@
> > > +        #No record found with this identifier
> > > +        if partial is None:
> > > +            logging.info('Request for invalid partial')
> > > +            resp = flask.Response("{result: '%s'}" %
> > > +                        "Partial with %s not found" % identifier,
> > 
> > Should be "Partial with identifier %s not found".
(Assignee)

Comment 13

3 years ago
(In reply to Anhad Jai Singh (:ffledgling) from comment #12)
> (In reply to Harsimran Singh from comment #11)
> > (In reply to Anhad Jai Singh (:ffledgling) from comment #10)
> > > Comment on attachment 8470147 [details] [diff] [review]
> > > 0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch
> > > 
> > > Review of attachment 8470147 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > This patch looks good for the most part.
> > > Giving this patch an r-, because there's a couple of nits that need to be
> > > fixed that I've pointed out inline.
> > > 
> > > Make sure to run all the integration-test and the curl-test to make sure we
> > > don't break something else accidentally.
> > 
> > i ran integration test and the test failed. here is the output
> > http://pastebin.com/d3Nm7QjJ for your reference. i hope this is the way
> > integration tests need to be run.
> 
> The test looks like it ran correctly for the most part, but there is a known
> issue with hashes on different operating systems, try passing a `-t` flag to
> the script and see if the test passes.

Sadly running with '-t' didn`t help either. Still the same error.

> If not, let me know and we can file a bug.
> 
> > > If you have your own github that you work on, you can use the .travs.yml
> > > present in the repository to run the tests automatically on every push.
> > > (This will help catch errors and help us give us more confidence that we
> > > don' break things.
> > > 
> > 
> > i have no idea how to use this ?
> You'll need a couple to do a couple of things to get this setup, you can
> skip it if you want to, but I'll list out the steps incase you're interested.
> (You can work with this next bug onwards in-case you're interested or not
> use it at all if you prefer that)

yes i`ll use it with the next bug.
> 
> 1. You'll need a github.com account.
> 2. You'll need to fork the build-funsize repository to your own account
> 3. You'll need to clone your fork of the github repo to your local machine
> 4. Go to travis.org, and follow steps #1 and #2here --
> http://docs.travis-ci.com/user/getting-started/
> 5. Make changes to your branch, once you're done, push your changes to
> github.
> 6. You should be able to see your test running at travis-ci.org if you did
> everything correctly.
> 
> If you need more info regarding travis-ci , see http://docs.travis-ci.com/
> and look for the relevant section.
> 
> As always if you have any questions or need any clarifications feel free to
> ask in IRC.
> 
> > 
> > > 
> > > Good work with this one.
> > > Make the needed changes and re-submit for review, and hopefully we'll be
> > > able to check this in!
> > > 
> > > ::: senbonzakura/frontend/api.py
> > > @@ +166,2 @@
> > > >      else:
> > > > +        #No record found with this identifier
> > > 
> > > Space between '#' and 'No' keeping with the style throughout the rest of the
> > > code.
> > > 
> > > @@ +167,5 @@
> > > > +        #No record found with this identifier
> > > > +        if partial is None:
> > > > +            logging.info('Request for invalid partial')
> > > > +            resp = flask.Response("{result: '%s'}" %
> > > > +                        "Partial with %s not found" % identifier,
> > > 
> > > Should be "Partial with identifier %s not found".
(Reporter)

Comment 14

3 years ago
(In reply to Harsimran Singh from comment #13)
> (In reply to Anhad Jai Singh (:ffledgling) from comment #12)
> > (In reply to Harsimran Singh from comment #11)
> > > (In reply to Anhad Jai Singh (:ffledgling) from comment #10)
> > > > Comment on attachment 8470147 [details] [diff] [review]
> > > > 0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch
> > > > 
> > > > Review of attachment 8470147 [details] [diff] [review]:
> > > > -----------------------------------------------------------------
> > > > 
> > > > This patch looks good for the most part.
> > > > Giving this patch an r-, because there's a couple of nits that need to be
> > > > fixed that I've pointed out inline.
> > > > 
> > > > Make sure to run all the integration-test and the curl-test to make sure we
> > > > don't break something else accidentally.
> > > 
> > > i ran integration test and the test failed. here is the output
> > > http://pastebin.com/d3Nm7QjJ for your reference. i hope this is the way
> > > integration tests need to be run.
> > 
> > The test looks like it ran correctly for the most part, but there is a known
> > issue with hashes on different operating systems, try passing a `-t` flag to
> > the script and see if the test passes.
> 
> Sadly running with '-t' didn`t help either. Still the same error.

Filed a bug, it was on my radar for a while, but now we have a bug for it --
Bug 1051617
> 
> > If not, let me know and we can file a bug.
> > 
> > > > If you have your own github that you work on, you can use the .travs.yml
> > > > present in the repository to run the tests automatically on every push.
> > > > (This will help catch errors and help us give us more confidence that we
> > > > don' break things.
> > > > 
> > > 
> > > i have no idea how to use this ?
> > You'll need a couple to do a couple of things to get this setup, you can
> > skip it if you want to, but I'll list out the steps incase you're interested.
> > (You can work with this next bug onwards in-case you're interested or not
> > use it at all if you prefer that)
> 
> yes i`ll use it with the next bug.
> > 
> > 1. You'll need a github.com account.
> > 2. You'll need to fork the build-funsize repository to your own account
> > 3. You'll need to clone your fork of the github repo to your local machine
> > 4. Go to travis.org, and follow steps #1 and #2here --
> > http://docs.travis-ci.com/user/getting-started/
> > 5. Make changes to your branch, once you're done, push your changes to
> > github.
> > 6. You should be able to see your test running at travis-ci.org if you did
> > everything correctly.
> > 
> > If you need more info regarding travis-ci , see http://docs.travis-ci.com/
> > and look for the relevant section.
> > 
> > As always if you have any questions or need any clarifications feel free to
> > ask in IRC.
> > 
> > > 
> > > > 
> > > > Good work with this one.
> > > > Make the needed changes and re-submit for review, and hopefully we'll be
> > > > able to check this in!
> > > > 
> > > > ::: senbonzakura/frontend/api.py
> > > > @@ +166,2 @@
> > > > >      else:
> > > > > +        #No record found with this identifier
> > > > 
> > > > Space between '#' and 'No' keeping with the style throughout the rest of the
> > > > code.
> > > > 
> > > > @@ +167,5 @@
> > > > > +        #No record found with this identifier
> > > > > +        if partial is None:
> > > > > +            logging.info('Request for invalid partial')
> > > > > +            resp = flask.Response("{result: '%s'}" %
> > > > > +                        "Partial with %s not found" % identifier,
> > > > 
> > > > Should be "Partial with identifier %s not found".
(Assignee)

Comment 15

3 years ago
(In reply to Anhad Jai Singh (:ffledgling) from comment #14)
> (In reply to Harsimran Singh from comment #13)
> > (In reply to Anhad Jai Singh (:ffledgling) from comment #12)
> > > (In reply to Harsimran Singh from comment #11)
> > > > (In reply to Anhad Jai Singh (:ffledgling) from comment #10)
> > > > > Comment on attachment 8470147 [details] [diff] [review]
> > > > > 0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch
> > > > > 
> > > > > Review of attachment 8470147 [details] [diff] [review]:
> > > > > -----------------------------------------------------------------
> > > > > 
> > > > > This patch looks good for the most part.
> > > > > Giving this patch an r-, because there's a couple of nits that need to be
> > > > > fixed that I've pointed out inline.
> > > > > 
> > > > > Make sure to run all the integration-test and the curl-test to make sure we
> > > > > don't break something else accidentally.
> > > > 
> > > > i ran integration test and the test failed. here is the output
> > > > http://pastebin.com/d3Nm7QjJ for your reference. i hope this is the way
> > > > integration tests need to be run.
> > > 
> > > The test looks like it ran correctly for the most part, but there is a known
> > > issue with hashes on different operating systems, try passing a `-t` flag to
> > > the script and see if the test passes.
> > 
> > Sadly running with '-t' didn`t help either. Still the same error.
> 
> Filed a bug, it was on my radar for a while, but now we have a bug for it --
> Bug 1051617
> > 

is there a workaround or i should wait for this Bug 1051617 to be fixed ?
(Reporter)

Comment 16

3 years ago
(In reply to Harsimran Singh from comment #15)

> is there a workaround or i should wait for this Bug 1051617 to be fixed ?

Go ahead and submit the patch, I'll take care of making sure we don't break something.
(Assignee)

Comment 17

3 years ago
Created attachment 8470782 [details] [diff] [review]
0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch

here is the patch with the fixes.
Attachment #8470147 - Attachment is obsolete: true
Attachment #8470782 - Flags: review?(ffledgling)
(Reporter)

Comment 18

3 years ago
Comment on attachment 8470782 [details] [diff] [review]
0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch

Review of attachment 8470782 [details] [diff] [review]:
-----------------------------------------------------------------

Perfect patch, the tests pass, you can see them here -- (https://travis-ci.org/mozilla/build-funsize/builds/32247919)

Congratulations, your changes are now committed and in the main repository!
Good work!

https://github.com/mozilla/build-funsize/commit/b059989e7067bc5913d7d1d58af0063b6cbf255a
Attachment #8470782 - Flags: review?(ffledgling) → review+
(Assignee)

Comment 19

3 years ago
(In reply to Anhad Jai Singh (:ffledgling) from comment #18)
> Comment on attachment 8470782 [details] [diff] [review]
> 0001-Bug-1040887-Funsize-returns-errors-out-and-returns-a.patch
> 
> Review of attachment 8470782 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Perfect patch, the tests pass, you can see them here --
> (https://travis-ci.org/mozilla/build-funsize/builds/32247919)
> 
> Congratulations, your changes are now committed and in the main repository!
> Good work!
> 

Thanks !

any suggestions on what to work on next ?

> https://github.com/mozilla/build-funsize/commit/
> b059989e7067bc5913d7d1d58af0063b6cbf255a
(Reporter)

Comment 20

3 years ago
(In reply to Harsimran Singh from comment #19)
> any suggestions on what to work on next ?

There are a couple of bugs I have, but they're probably a little harder than the previous ones.
The I think you might be able to work on: Bug 1048753

You can also search bugzilla for the 'funsize' tag and see if you come across anything interesting.

If none of these interest you, feel free to drop by #releng in IRC and ask someone for a bug, but you might have to idle around on the channel a bit before you get a response depending on the time.

Thanks again for helping out!
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(In reply to Harsimran Singh from comment #19)
> 
> any suggestions on what to work on next ?

Hey Harsimran,

I've created a search for RelEng "good first bugs":
https://bugzilla.mozilla.org/buglist.cgi?cmdtype=dorem&remaction=run&namedcmd=RelEng%20good%20first%20bugs&sharer_id=464696

If nothing gets you excited, here is a larger set, though may contain some rather tricky ones that might require a lot of knowledge about our setup:
https://bugzilla.mozilla.org/buglist.cgi?cmdtype=dorem&remaction=run&namedcmd=Unresolved%20bugs%20raised%20by%20pmoore&sharer_id=464696&list_id=10997357

Let me know if anything tickles your fancy.

Pete
Flags: needinfo?(harsimran_hs4)
(Assignee)

Comment 22

3 years ago
(In reply to Pete Moore [:pete][:pmoore] from comment #21)
> (In reply to Harsimran Singh from comment #19)
> > 
> > any suggestions on what to work on next ?
> 
> Hey Harsimran,
> 
> I've created a search for RelEng "good first bugs":
> https://bugzilla.mozilla.org/buglist.
> cgi?cmdtype=dorem&remaction=run&namedcmd=RelEng%20good%20first%20bugs&sharer_
> id=464696
> 
> If nothing gets you excited, here is a larger set, though may contain some
> rather tricky ones that might require a lot of knowledge about our setup:
> https://bugzilla.mozilla.org/buglist.
> cgi?cmdtype=dorem&remaction=run&namedcmd=Unresolved%20bugs%20raised%20by%20pm
> oore&sharer_id=464696&list_id=10997357
> 
> Let me know if anything tickles your fancy.
> 

Thanks Pete for the search links.
Currently i am working on Bug 1048753 and will let you know once i finish it.
 

> Pete
Flags: needinfo?(harsimran_hs4)
You need to log in before you can comment on or make changes to this bug.