fix: replaced taskcluster rest api calls with package

[abhishekmadan30]

fixes #221 . Created a bunch of helpers to use this package in taskcluster.py . I also didnt completely depcreate the url creation as that is still used. I bypassed it and replaced all uses when to get request is made.

[ahal]

This is shaping up nicely, thanks!

I have some changes to request. Once you put up a new revision I'll pull this down locally and see if it causes any differences in the graph (both here and in Gecko).

[ahal]

Thanks! I'll take a closer look at this tomorrow. I'll try and test it against gecko_taskgraph as well.

[ahal]

Thanks, I tested this patch both in Taskgraph and the Gecko repo and it looks like it's working as expected!

But get_retry_post_session, _do_request and _handle_artifact are all unused now in taskcluster.py. Can you remove these and associated tests please?

[ahal]

Also I think I changed my mind about leaving use_proxy in the function signature just for backwards compatibility.. We should remove it rather than confuse consumers who are attempting to set it to False or whatnot and then wondering why it isn't working.

We can do a major version bump for this.. no big deal.

[ahal]

Thank you!

This is mostly cleanup, but there are two issues I neglected to catch in my first review:

1. The _handle_artifact for yaml
2. The .capitalize() issue

[ahal]

Are you sure the taskcluster client would return None on a 404? I would have thought it would raise an exception still.. Though it would probably be from aiohttp rather than requests.

[ahal]

(I assume you verified this? It's important to preserve the 404 behaviour here)

[ahal]

Thanks! Sorry for the delay + additional iteration. Getting close :)

[ahal]

I might be wrong, but I think if public/label-to-taskid.json is a 404 then it would raise an exception rather than return None. So this else clause is probably dead code.

[ahal]

Did these type failures start happening as a result of your changes? Or do you get these errors even on main? If the latter, I wonder if there's something you need to do locally to get the type checker working

[abhishekmadan30]

These happened because of my change because currently task_def could be a json or a responce if it fails

[ahal]

Ditto

[ahal]

Re-raising the exception without doing anything is the same as not catching it in the first place. So you might as well remove the try/except block.

[ahal]

(I assume you verified this? It's important to preserve the 404 behaviour here)

[ahal]

Thank you, lgtm!

I think schema change is close enough to landing that maybe we should land this and that at roughly the same time so we can avoid requiring two separate major version bumps.