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

Set component id if a numeric component is available in the component… #4008

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alhijazi
Copy link
Collaborator

… list

@alhijazi alhijazi requested a review from rmistry May 27, 2024 15:28
@alhijazi
Copy link
Collaborator Author

@rmistry is there a way to get a componentid from component tag?
Does predator return component tags or ids?

# Set the componentId to the first encountered numeric component
# in the component list.
for component in list(self.components):
if component.isnumeric():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm...will component ever be an int? this will fail in that case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will not be an int, we are doing the same thing a few lines above:

component_paths = self._get_component_paths(self.components)

@rmistry
Copy link
Collaborator

rmistry commented May 28, 2024

@rmistry is there a way to get a componentid from component tag? Does predator return component tags or ids?

Interesting bugs for this area are b/315853316 and b/318504348
Interesting PRs for this area are #3584 and #3612

At the time those were submitted, predator returned full component paths, but infrastructure was added to make sure that whenever Chromium's DIR_METADATA was updated to contain component_ids it would continue to work. I do not know whether that was done or not. Looking at cs.chromium.org it looks like all the random ones I clicked on still had component paths (not ids).

I don't think you can get a component_id from a component_name in issuetracker's public API (I could not find a way).

# in the component list.
for component in list(self.components):
if component.isnumeric():
self._data['issueState']['componentId'] = int(component)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to this it will set the first component that predator returns. It would be worth looking in logs for components in predator responses to see what is being returned. Maybe the first is not what we want, but it is likely impossible to say what we want.
Perhaps a better way to handle this is to say iff the size of self.components is 1 then set the componentId, otherwise leave it alone.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though as mentioned in b/341800538#comment8 the PEEPs bot is probably setting things to the 1st component anyway (would be good to verify what actually happens).

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.

3 participants