fix: added use_proxy flag to get_artifact_url #807
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hneiva
reviewed
Oct 7, 2025
| def get_artifact_url(task_id, path): | ||
| artifact_tmpl = liburls.api(get_root_url(), "queue", "v1", "task/{}/artifacts/{}") | ||
| def get_artifact_url(task_id, path, use_proxy=False): | ||
| if use_proxy: |
Contributor
There was a problem hiding this comment.
I wonder if we should bypass this flag if TASKCLUSTER_PROXY_URL is not in os.environ.
aka make it if use_proxy and "TASKCLUSTER_PROXY_URL" in os.environ:
Contributor
Author
There was a problem hiding this comment.
I think it would be better to raise the error as instead if they are expecting a proxy to be use but the environment variable isn't set I think we should throw an error
abhishekmadan30
force-pushed
the
artifact-url
branch
2 times, most recently
from
a4f4124 to
f129b62
Compare
ahal
approved these changes
Oct 8, 2025
src/taskgraph/util/taskcluster.py
Outdated
| else: | ||
| artifact_tmpl = liburls.api( | ||
| get_root_url(block_proxy=True), "queue", "v1", "task/{}/artifacts/{}" | ||
| ) |
Collaborator
There was a problem hiding this comment.
You can move the liburls.api() call out of this if statement, then only set the url here to de-dupe a bit.
abhishekmadan30
force-pushed
the
artifact-url
branch
from
f129b62 to
200795b
Compare
abhishekmadan30
force-pushed
the
artifact-url
branch
from
200795b to
74e1af9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I thought adding a block_proxy flag to get_root_url, would be the best option for get_rool_url so we could avoid having to duplicate the code from get_root_url if use_proxy is not used in get_artifact_url without it accidently picking up the proxy url even if that is set to false