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.

Sign up for GitHub

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

Jump to bottom

feat(PAT): Add personal access token for programmatic access #2261

Merged
merged 6 commits into from
Mar 2, 2023
Merged

feat(PAT): Add personal access token for programmatic access #2261

merged 6 commits into from
Mar 2, 2023

Conversation

ahsanbarkati
Copy link
Contributor

Add the DAO methods for the following:

  • CreatePAT
  • GetPAT
  • ListPATs
  • DeletePAT

Create corresponding HTTP APIs.

As of now, the expiry field of the personal access token is unused.

@ahsanbarkati ahsanbarkati changed the title feat(PAT): Add DAOs and APIs for personal access token CRUD operations [WIP] feat(PAT): Add DAOs and APIs for personal access token CRUD operations Feb 15, 2023
@ahsanbarkati ahsanbarkati changed the title [WIP] feat(PAT): Add DAOs and APIs for personal access token CRUD operations feat(PAT): Add personal access token based authorisation Feb 15, 2023
@ahsanbarkati ahsanbarkati changed the title feat(PAT): Add personal access token based authorisation feat(PAT): Add personal access token for programmatic access Feb 15, 2023
mindhash
mindhash reviewed Feb 22, 2023
View reviewed changes
Copy link
Contributor

@mindhash mindhash left a comment

Choose a reason for hiding this comment

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

overall LGTM.

Most comments for just food for further thought. So do take them into consideration and implement if you feel they make sense.

The only must have fix is deletePAT -> GetPat() call. It needs a fix i think.

Thanks!

ee/query-service/app/api/pat.go Show resolved Hide resolved
ee/query-service/app/api/pat.go Outdated Show resolved Hide resolved
ee/query-service/app/api/pat.go Outdated Show resolved Hide resolved
ee/query-service/app/server.go Outdated Show resolved Hide resolved
ee/query-service/app/server.go Outdated Show resolved Hide resolved
ee/query-service/app/api/pat.go Outdated Show resolved Hide resolved
ee/query-service/app/server.go Outdated Show resolved Hide resolved
ee/query-service/app/server.go Show resolved Hide resolved
ee/query-service/dao/sqlite/modelDao.go Show resolved Hide resolved
mindhash
mindhash previously approved these changes Feb 27, 2023
View reviewed changes
@sonarcloud
Copy link

sonarcloud bot commented Feb 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ankitnayan ankitnayan merged commit b99d700 into SigNoz:develop Mar 2, 2023
srikanthccv added a commit that referenced this pull request Mar 3, 2023
@srikanthccv
fix: add the missing /health route removed in #2261
bc119b6
ankitnayan pushed a commit that referenced this pull request Mar 3, 2023
@srikanthccv
fix: add the missing /health route removed in #2261 (#2419)
2c7cefc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srikanthccv srikanthccv srikanthccv left review comments

@mindhash mindhash mindhash left review comments

@ankitnayan ankitnayan Awaiting requested review from ankitnayan

@prashant-shahi prashant-shahi Awaiting requested review from prashant-shahi

@palashgdev palashgdev Awaiting requested review from palashgdev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ahsanbarkati @mindhash @ankitnayan @srikanthccv