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

Perform query lookup for the specific target in case it's not part of the synced data #4547

Closed
wants to merge 1 commit into from

Conversation

rogerhu
Copy link
Contributor

@rogerhu rogerhu commented Mar 4, 2023

Related to: #4546

Checklist

  • I have filed an issue about this change and discussed potential changes with the maintainers.
  • I have received the approval from the maintainers to make this change.
  • This is not a stylistic, refactoring, or cleanup change.

Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.

Discussion thread for this change

Issue number: #4546

Description of this change

If the kind cannot be determined from the sync process, we invoke the Bazel process to query the kind being specified. There are similar classes that do similar things but this code gets invoked asynchronously as a Future and we only want to query the target specifically.

@rogerhu rogerhu changed the title Temp patch to do query lookup for the specific target Perform query lookup for the specific target in case it's not part of the synced data Mar 4, 2023
@rogerhu rogerhu force-pushed the rogerh/ISSUE-4546 branch 5 times, most recently from 204b686 to e8cc5e0 Compare March 5, 2023 00:09
@sgowroji sgowroji added product: IntelliJ IntelliJ plugin awaiting-review Awaiting review from Bazel team on PRs labels Mar 5, 2023
@cheister
Copy link

cheister commented Mar 8, 2023

+1

@mai93
Copy link
Collaborator

mai93 commented Mar 8, 2023

IIUC Finder interfaces like TargetFinder and SourceToTargetFinder must be fast, that's why they are implemented to just lookup the TargetMap. Also I think there is a pattern to avoid running bazel commands, like bazel query in this case, implicitly like this to not affect the performance.

@rogerhu
Copy link
Contributor Author

rogerhu commented Mar 9, 2023

IIUC Finder interfaces like TargetFinder and SourceToTargetFinder must be fast, that's why they are implemented to just lookup the TargetMap. Also I think there is a pattern to avoid running bazel commands, like bazel query in this case, implicitly like this to not affect the performance.

If that's a requirement (or is it?), would removing the manual tag (

return String.format("attr(\"tags\", \"^((?!manual).)*$\", %s)", targets);
) as part of generating the TargetMap be a better alternative?

@mai93
Copy link
Collaborator

mai93 commented Jul 28, 2023

@rogerhu can we close this PR as #5085 is merged now?

@rogerhu
Copy link
Contributor Author

rogerhu commented Jul 28, 2023

Yes!

@rogerhu rogerhu closed this Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review Awaiting review from Bazel team on PRs product: IntelliJ IntelliJ plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants