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

Change search queries to GraphQl to avoid permission issues #70

Closed
gentlementlegen opened this issue Oct 28, 2024 · 10 comments · Fixed by #71
Closed

Change search queries to GraphQl to avoid permission issues #70

gentlementlegen opened this issue Oct 28, 2024 · 10 comments · Fixed by #71

Comments

@gentlementlegen
Copy link
Member

          It is still weird to me that the user privacy affects a search because the profile is public. Can we consider using GQL with issues search instead of the search API? Something like
query($organization: String!, $author: String!) {
  organization(login: $organization) {
    repositories(first: 100) {
      nodes {
        issues(first: 100, states: OPEN, filterBy: {createdBy: $author}) {
          nodes {
            title
            url
            createdAt
          }
        }
      }
    }
  }
}

with

{
  "organization": "ubiquity",
  "author": "sshivaditya2019"
}

would achieve the same result. I don't know if that would resolve the issue but it's worth a try.

Originally posted by @gentlementlegen in ubiquity/work.ubq.fi#119 (comment)

@0x4007
Copy link
Member

0x4007 commented Oct 28, 2024

Somebody should test and see if this works before we fund it.

@gentlementlegen gentlementlegen self-assigned this Oct 28, 2024
Copy link

@gentlementlegen the deadline is at Mon, Oct 28, 3:49 PM UTC

@gentlementlegen
Copy link
Member Author

@0x4007 I spent quite some time on this. It seems to work ok but it is awfully slow at the moment, I'll try to see if there is a better way.

@gentlementlegen
Copy link
Member Author

gentlementlegen commented Oct 29, 2024

@0x4007 I managed to make it faster but it takes still something like 5 to 10 seconds depending on the org (ubiquity for example has tons of PRs). GitHub search is obviously using caching and indexing to provide near instant results. Maybe we could have this logic as a backup when the search fails?

Sidenote, this seems to happen when the user has set his activity to private.

@0x4007
Copy link
Member

0x4007 commented Oct 29, 2024

Seamless fallback seems like a reasonable solution.

Copy link

ubiquity-os-beta bot commented Oct 30, 2024

 [ 115.938 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueTask1100
IssueSpecification111.25
IssueComment24.688
ReviewComment20
Conversation Incentives
CommentFormattingRelevanceReward
It is still weird to me that the user privacy affects a search b…
3.75
content:
  content:
    p:
      score: 0
      elementCount: 4
    em:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 71
  wordValue: 0.1
  result: 3.75
111.25
@0x4007 I spent quite some time on this. It seems to work ok but…
1.9
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 32
  wordValue: 0.1
  result: 1.9
0.81.52
@0x4007 I managed to make it faster but it takes still something…
3.52
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 66
  wordValue: 0.1
  result: 3.52
0.93.168
Resolves #70QA: https://github.com/Meniole/command-start-stop/…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 20
  wordValue: 0
  result: 0
0.20
@0x4007 On the good news: it works, I successfully retrieve the …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 53
  wordValue: 0
  result: 0
0.70

 [ 1.028 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment21.028
Conversation Incentives
CommentFormattingRelevanceReward
Somebody should test and see if this works before we fund it.
0.83
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 12
  wordValue: 0.1
  result: 0.83
0.80.664
Seamless fallback seems like a reasonable solution.
0.52
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 7
  wordValue: 0.1
  result: 0.52
0.70.364

Copy link

! chat_not_found

@gentlementlegen
Copy link
Member Author

@Keyrxng We might want to improve that message, maybe not even post it because I don't think it's relevant to users.

@Keyrxng
Copy link
Contributor

Keyrxng commented Oct 30, 2024

I recently added logic to comment any status !== 200 for workflows during debugging it's handy but needs tweaked for sure.

For close we should do nothing. For reopen, should we create one or do nothing or detect a merged PR and only open if there isn't one?

Some little behaviours need refined and tweaked

@gentlementlegen
Copy link
Member Author

I think room creation failure is important to understand that it tried to open it but failed; closing errors might be silent. We always have the logs in the Actions and Cloudflare logs if needed. But yes we should discuss these in details. We can keep the closing error but we need a prettier message at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants