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(addon-logs-archives): add logs archives for addons #694

Merged
merged 4 commits into from
Nov 5, 2021
Merged

feat(addon-logs-archives): add logs archives for addons #694

merged 4 commits into from
Nov 5, 2021

Conversation

sihamais
Copy link
Member

@sihamais sihamais commented Nov 3, 2021 •
edited by EtienneM
Loading

Waiting for #701 to be merged

This PR also contains a couple of files renaming and functions renaming. You can filter out these commits to better see what this PR contains.

  • Add a changelog entry in the section "To Be Released" of CHANGELOG.md
  • Test it when AddonLogsArchives is released on go-scalingo#235

Fix #518

@sihamais sihamais marked this pull request as draft November 3, 2021 15:47
@sihamais
Copy link
Member Author

sihamais commented Nov 3, 2021

  1. I made a first draft for this feature but I have no way of testing it since I don't have any addon archive on my project. For now it tells me well that I have no archive. But I would like to test it more. Is there a way ?
  2. What's the difference between the page and the cursor flag ? And there is a function to get app logs archives with cursor flag but it isn't called anywhere (I think). So where is it used ?
    @EtienneM

@EtienneM
Copy link
Member

EtienneM commented Nov 4, 2021

  1. You should have an access to a staging app with database logs archives available. Is it OK?
  2. I don't understand where you see this cursor thing. Can you clarify? About the logs archives for app with a cursor flag, can you point me to a file?

@sihamais
Copy link
Member Author

sihamais commented Nov 4, 2021

  1. Yep, it's good.
  2. I've seen it in the request URL on the issue, and i've checked that there is a cursor param on the request made on the dashboard. Also, since app logs archives use it I figured addons logs archives would use it too.

@sihamais
Copy link
Member Author

sihamais commented Nov 4, 2021

Actually, the function I've seen is in another project https://github.com/Scalingo/go-scalingo/blob/master/logs-archives.go#L33
(I got lost) Still don't know when cursor is used ? Should I just ignore it and use the page argument ?

@sihamais sihamais requested a review from EtienneM November 4, 2021 10:22
@EtienneM
Copy link
Member

EtienneM commented Nov 4, 2021

The cursor is the old way we used to use for the pagination. The new way is the page parameter, so please use this one.

cursor was used by the old dashboard. Now that the old dashboard is deprecated, we will remove the support for cursor. Thanks to your report, I opened an issue on our side.

Note that go-scalingo is the Go client to interact with our API.

@EtienneM EtienneM removed their request for review November 4, 2021 10:35
@sihamais
Copy link
Member Author

sihamais commented Nov 4, 2021

Okay, so go-scalingo is the Go client to interact with the API but it does not have a method to get the logs archives of the addon (here https://github.com/Scalingo/go-scalingo/blob/master/logs-archives.go). I actually implemented it yesterday but I just realized that i was editing on the dependancy (got lost). What should I do ? Normally it should be on the go-scalingo project but I'm not sure I have the right to open a PR there and even if I do I would have to wait for the next release to use it in the CLI. I'm kinda stuck here.

@sihamais sihamais requested a review from EtienneM November 4, 2021 11:16
@EtienneM
Copy link
Member

EtienneM commented Nov 4, 2021

The process is indeed to develop this feature on go-scalingo and open a PR there. I'm the one in charge of the go-scalingo release, so I'll proceed with a new release as soon as I merge your PR. That's what we usually do. You can see that in the changelog, many versions contain a single entry (https://github.com/Scalingo/go-scalingo/blob/master/CHANGELOG.md).

@EtienneM EtienneM removed their request for review November 4, 2021 13:14
@EtienneM EtienneM force-pushed the feat/518/addon-logs-archive branch 7 times, most recently from 7352a9d to 801c33c Compare November 5, 2021 15:14
@EtienneM EtienneM force-pushed the feat/518/addon-logs-archive branch from 801c33c to 56accc2 Compare November 5, 2021 15:17
@EtienneM EtienneM marked this pull request as ready for review November 5, 2021 15:19
curzolapierre
curzolapierre approved these changes Nov 5, 2021
View reviewed changes
Copy link
Member

@curzolapierre curzolapierre left a comment

Choose a reason for hiding this comment

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

SGTM

db/logs_archive.go
Comment on lines +37 to +42
fmt.Println(color.New(color.FgYellow).Sprint("To: ") + archive.To)
fmt.Println(color.New(color.FgYellow).Sprint("From: ") + archive.From)
fmt.Println(color.New(color.FgYellow).Sprint("Size: ") + strconv.FormatInt(archive.Size, 10))
fmt.Println(color.New(color.FgYellow).Sprint("Url: ") + archive.Url)
fmt.Println("-------")
}
Copy link
Member

Choose a reason for hiding this comment

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

Do you have an example of the output ?

Copy link
Member

Choose a reason for hiding this comment

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

To:   Fri Nov 5 13:10:56 +0000 UTC 2021
From: Fri Nov 5 09:15:53 +0000 UTC 2021
Size: 3705928                          
Url:  https://logs-archive-service-osc-fr1.scalingo.com/logs-archives/[REDACTED]/download/[REDACTED].log-2021110513.gz?sign=[REDACTED]&exp=1636131525               
-------                                                                                                                                                                                                            To:   Fri Nov 5 09:15:51 +0000 UTC 2021
From: Fri Nov 5 05:15:12 +0000 UTC 2021                                                                                                                                                                            
Size: 3551531                                                                                                                                                                                                      
Url:  https://logs-archive-service-osc-fr1.scalingo.com/logs-archives/[REDACTED]/download/[REDACTED].log-2021110509.gz?sign=[REDACTED]&exp=1636131525
-------
To:   Fri Nov 5 05:15:12 +0000 UTC 2021
From: Fri Nov 5 01:15:56 +0000 UTC 2021
Size: 3436225
Url:  https://logs-archive-service-osc-fr1.scalingo.com/logs-archives/[REDACTED]/download/[REDACTED].log-2021110505.gz?sign=[REDACTED]&exp=1636131525

@EtienneM EtienneM merged commit 41d3b36 into master Nov 5, 2021
@EtienneM EtienneM deleted the feat/518/addon-logs-archive branch November 5, 2021 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EtienneM EtienneM EtienneM left review comments

@curzolapierre curzolapierre curzolapierre approved these changes

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

Successfully merging this pull request may close these issues.

[Logs archive] Make it available for addons
3 participants
@sihamais @EtienneM @curzolapierre