-
Notifications
You must be signed in to change notification settings - Fork 729
Github archive #2691
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
Github archive #2691
Changes from 25 commits
0412c35
5705571
040b95c
d96883f
9c7b4b0
7200b42
164240a
c9fbc20
a5d30d4
41d3353
a5b8aae
2d59559
2377e64
d2371e7
62d222d
edb5c80
aedbb35
84ce248
dffbfac
bdfd13a
a74fad9
f18dd04
b11fc25
ddafb39
45eae0f
189c093
c0b1616
3b1276e
8ed8aac
7ec4541
6639a40
6541967
c1a7b09
66d482a
408c155
2d4e896
bf609e8
4b36d45
c5272f7
9c55d68
416218b
b380ae4
6c00277
a26afcf
fde1f73
f920447
c3c45dd
9acea9d
a8d814c
244da57
3889452
18c305a
cceceda
0851772
ce902ae
323ab88
06a7850
bcf88d1
acbcc05
bc722de
ddbdc1f
d1bb5b6
ce32d7d
7e84fcd
da1ef1b
7e878b2
f03246f
6b5e8c9
fbc9961
6e81949
1e333c1
48b4bdb
ee0b036
e343209
2eb7db7
999b266
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import Permissions from '@/security/permissions' | ||
| import GithubIntegrationService from '@/services/githubIntegrationService' | ||
| import PermissionChecker from '@/services/user/permissionChecker' | ||
|
|
||
| export default async (req, res) => { | ||
| new PermissionChecker(req).validateHas(Permissions.values.integrationEdit) | ||
|
|
||
| const payload = await GithubIntegrationService.getOrgRepos(req.params.org) | ||
| await req.responseHandler.success(req, res, payload) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import Permissions from '@/security/permissions' | ||
| import GithubIntegrationService from '@/services/githubIntegrationService' | ||
| import PermissionChecker from '@/services/user/permissionChecker' | ||
|
|
||
| export default async (req, res) => { | ||
| new PermissionChecker(req).validateHas(Permissions.values.integrationEdit) | ||
|
|
||
| const payload = await GithubIntegrationService.findOrgs(req.query.query) | ||
| await req.responseHandler.success(req, res, payload) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import Permissions from '@/security/permissions' | ||
| import GithubIntegrationService from '@/services/githubIntegrationService' | ||
| import PermissionChecker from '@/services/user/permissionChecker' | ||
|
|
||
| export default async (req, res) => { | ||
| new PermissionChecker(req).validateHas(Permissions.values.integrationEdit) | ||
|
|
||
| const payload = await GithubIntegrationService.findGithubRepos(req.query.query) | ||
| await req.responseHandler.success(req, res, payload) | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { request } from '@octokit/request' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { GithubSnowflakeClient, SnowflakeClient } from '@crowd/snowflake' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IServiceOptions } from './IServiceOptions' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default class GithubIntegrationService { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(private readonly options: IServiceOptions) {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public async getGithubRepositories(org: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const client = SnowflakeClient.fromEnv() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.options.log.info(`Getting GitHub repositories for org: ${org}`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const githubClient = new GithubSnowflakeClient(client) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return githubClient.getOrgRepositories({ org, perPage: 10000 }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static async findGithubRepos(query: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [orgRepos, repos] = await Promise.all([ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request('GET /search/repositories', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| q: `owner:${query}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }).catch((err) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.options.log.error(`Error getting GitHub repositories for org: ${query}`, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { data: { items: [] } } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request('GET /search/repositories', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| q: query, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // console.log([...orgRepos.data.items, ...repos.data.items]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return [...orgRepos.data.items, ...repos.data.items].map((item) => ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: item.name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url: item.html_url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| org: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: item.owner.login, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url: item.owner.html_url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logo: item.owner.avatar_url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve error handling and reduce code duplication
+ private createGitHubRequest(endpoint: string, params: object) {
+ return request(endpoint, {
+ ...params,
+ headers: {
+ authorization: `bearer ${GITHUB_TOKEN_CONFIG.token}`,
+ },
+ });
+ }
public async findGithubRepos(query: string) {
- const auth = GITHUB_TOKEN_CONFIG.token
+ try {
const [orgRepos, repos] = await Promise.all([
- request('GET /search/repositories', {
- q: `owner:${query}`,
- headers: {
- authorization: `bearer ${auth}`,
- }
- }).catch((err) => {
- this.options.log.error(`Error getting GitHub repositories for org: ${query}`, err)
- return { data: { items: [] } }
- }),
+ this.createGitHubRequest('GET /search/repositories', { q: `owner:${query}` }),
+ this.createGitHubRequest('GET /search/repositories', { q: query })
- request('GET /search/repositories', {
- q: query,
- headers: {
- authorization: `bearer ${auth}`,
- }
- }).catch((err) => {
- this.options.log.error(`Error getting GitHub repositories for org: ${query}`, err)
- return { data: { items: [] } }
- }),
])
return [...orgRepos.data.items, ...repos.data.items].map((item) => ({
name: item.name,
url: item.html_url,
org: {
name: item.owner.login,
url: item.owner.html_url,
logo: item.owner.avatar_url,
},
}))
+ } catch (error) {
+ this.options.log.error(`Failed to search GitHub repositories for query: ${query}`, error);
+ throw new GitHubIntegrationError(`Failed to search repositories for ${query}`, error);
+ }
}
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static async findOrgs(query: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await request('GET /search/users', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| q: query, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return response.data.items.map((item) => ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: item.login, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url: item.html_url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logo: item.avatar_url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+53
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Convert static method to instance method and add error handling
- public static async findOrgs(query: string) {
+ public async findOrgs(query: string) {
- const auth = GITHUB_TOKEN_CONFIG.token
+ try {
- const response = await request('GET /search/users', {
- q: query,
- headers: {
- authorization: `bearer ${auth}`,
- }
- })
+ const response = await this.createGitHubRequest('GET /search/users', { q: query });
return response.data.items.map((item) => ({
name: item.login,
url: item.html_url,
logo: item.avatar_url,
}))
+ } catch (error) {
+ this.options.log.error(`Failed to search GitHub organizations for query: ${query}`, error);
+ throw new GitHubIntegrationError(`Failed to search organizations for ${query}`, error);
+ }
}
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static async getOrgRepos(org: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await request('GET /orgs/{org}/repos', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| org, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return response.data.map((repo) => ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: repo.name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url: repo.html_url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+68
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Refactor static method and resolve potential duplication
Consider consolidating this with - public static async getOrgRepos(org: string) {
+ public async getOrgRepos(org: string) {
+ // TODO: Document why this method exists alongside getGithubRepositories
+ try {
- const auth = GITHUB_TOKEN_CONFIG.token
- const response = await request('GET /orgs/{org}/repos', {
- org,
- headers: {
- authorization: `bearer ${auth}`,
- }
- })
+ const response = await this.createGitHubRequest('GET /orgs/{org}/repos', { org });
return response.data.map((repo) => ({
name: repo.name,
url: repo.html_url,
}))
+ } catch (error) {
+ this.options.log.error(`Failed to get repositories for organization: ${org}`, error);
+ throw new GitHubIntegrationError(`Failed to get repositories for ${org}`, error);
+ }
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Optimize Snowflake client initialization and add error handling
export default class GithubIntegrationService { + private readonly snowflakeClient: SnowflakeClient; + private readonly githubSnowflakeClient: GithubSnowflakeClient; - constructor(private readonly options: IServiceOptions) {} + constructor(private readonly options: IServiceOptions) { + this.snowflakeClient = SnowflakeClient.fromEnv(); + this.githubSnowflakeClient = new GithubSnowflakeClient(this.snowflakeClient); + } public async getGithubRepositories(org: string) { - const client = SnowflakeClient.fromEnv() this.options.log.info(`Getting GitHub repositories for org: ${org}`) - const githubClient = new GithubSnowflakeClient(client) - return githubClient.getOrgRepositories({ org, perPage: 10000 }) + try { + return await this.githubSnowflakeClient.getOrgRepositories({ + org, + perPage: 1000 // Consider implementing pagination + }); + } catch (error) { + this.options.log.error(`Failed to get GitHub repositories for org: ${org}`, error); + throw new GitHubIntegrationError(`Failed to get repositories for ${org}`, error); + } }