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

list log drains by DB (from addon id) #572

Merged
merged 17 commits into from Jun 25, 2020

Conversation

curzolapierre
Copy link
Contributor

@curzolapierre curzolapierre commented Jun 12, 2020

Command: list log drains by DB (from addon uuid)

Example:

$ scalingo --app my-app log-drains --addon ad-9be0fc04-bee6-4981-a403-a9ddbee6bd1f
+----------------+---------------+------------------------------------------+
|   ADDON NAME   |  ADDON PLAN   |                 URL                      |
+----------------+---------------+------------------------------------------+
| Scalingo Redis | redis-sandbox | tcp+tls://logs.papertrailapp.com:10303   |
|                |               | ovh://:id@tag1.logs.ovh.com:6514         |
+----------------+---------------+------------------------------------------+

$ scalingo --app my-app log-drains --with-addon
+-----------------+------------------+----------------------------------------+
|   ADDON NAME    |    ADDON PLAN    |                 URL                    |
+-----------------+------------------+----------------------------------------+
| Scalingo Redis  | redis-sandbox    | tcp+tls://logs.papertrailapp.com:10303 |
+                 +                  +----------------------------------------+
|                 |                  | ovh://:id@tag1.logs.ovh.com:6514       |
+-----------------+------------------+----------------------------------------+
| Scalingo Mongo  | mongo-sandbox    | ovh://:id@tag1.logs.ovh.com:6514       |
+-----------------+------------------+----------------------------------------+

Fix #571

@curzolapierre curzolapierre requested a review from EtienneM Jun 15, 2020
@curzolapierre curzolapierre removed the request for review from EtienneM Jun 16, 2020
@curzolapierre curzolapierre marked this pull request as draft Jun 16, 2020
@curzolapierre curzolapierre force-pushed the feature/571/list_log_drains_by_DB branch from cb769f2 to ff1a8e8 Compare Jun 17, 2020
@curzolapierre curzolapierre force-pushed the feature/571/list_log_drains_by_DB branch from 07afbf5 to 8af3af9 Compare Jun 17, 2020
@curzolapierre curzolapierre marked this pull request as ready for review Jun 17, 2020
@curzolapierre curzolapierre requested a review from EtienneM Jun 17, 2020
Copy link
Member

@EtienneM EtienneM left a comment

+                 +                  +----------------------------------------+

This is odd that you have an extra line here when displaying multiple log drains whereas it's not here with the flag --addon

cmd/log_drains.go Outdated Show resolved Hide resolved
cmd/log_drains.go Show resolved Hide resolved
cmd/log_drains.go Outdated Show resolved Hide resolved
@curzolapierre
Copy link
Contributor Author

curzolapierre commented Jun 23, 2020

Add separation lines could be usefull in case of multiple addons + application in the table:

+-------------------------------+--------------------------------------------------------------------+
|             NAME              |                                URL                                 |
+-------------------------------+--------------------------------------------------------------------+
| sample-go-martini             | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
+                               +--------------------------------------------------------------------+
|                               | tcp+tls://logs.papertrailapp.com:10303                             |
+-------------------------------+--------------------------------------------------------------------+
| Scalingo Redis                | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
+                               +--------------------------------------------------------------------+
|                               | tcp+tls://logs.papertrailapp.com:10303                             |
+-------------------------------+--------------------------------------------------------------------+

And disable it in case of just one listing

+-------------------------------+--------------------------------------------------------------------+
|             NAME              |                                URL                                 |
+-------------------------------+--------------------------------------------------------------------+
| Scalingo Redis                | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
|                               | tcp+tls://logs.papertrailapp.com:10303                             |
+-------------------------------+--------------------------------------------------------------------+

Case without separation lines:

+-------------------------------+--------------------------------------------------------------------+
|             NAME              |                                URL                                 |
+-------------------------------+--------------------------------------------------------------------+
| sample-go-martini             | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
|                               | tcp+tls://logs.papertrailapp.com:10303                             |
| Scalingo Redis                | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
|                               | tcp+tls://logs.papertrailapp.com:10303                             |
+-------------------------------+--------------------------------------------------------------------+

@curzolapierre curzolapierre force-pushed the feature/571/list_log_drains_by_DB branch from dbcbdbf to 710a261 Compare Jun 23, 2020
@curzolapierre curzolapierre requested a review from EtienneM Jun 23, 2020
Copy link
Member

@EtienneM EtienneM left a comment

I think I prefer the following solution:

+-------------------------------+--------------------------------------------------------------------+
|             NAME              |                                URL                                 |
+-------------------------------+--------------------------------------------------------------------+
| sample-go-martini             | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
|                               | tcp+tls://logs.papertrailapp.com:10303                             |
+-------------------------------+--------------------------------------------------------------------+
| Scalingo Redis                | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
|                               | tcp+tls://logs.papertrailapp.com:10303                             |
+-------------------------------+--------------------------------------------------------------------+

What do you think?

log_drains/list_addon.go Outdated Show resolved Hide resolved
log_drains/list_addon.go Outdated Show resolved Hide resolved
log_drains/list_addon.go Outdated Show resolved Hide resolved
@curzolapierre
Copy link
Contributor Author

curzolapierre commented Jun 24, 2020

I would prefer your example but it seems tablewriter doesn't offer the possibility to do it.
So I tried manually, what do you think ? Maybe the implementation is too 'dirty' for the result:

+-------------------------------+--------------------------------------------------------------------+
|             NAME              |                                URL                                 |
+-------------------------------+--------------------------------------------------------------------+
| Scalingo Redis                | tcp+tls://logs.papertrailapp.com:10303                             |
| ----------------------------- | ------------------------------------------------------------------ |
| Scalingo Redis                | tcp+tls://logs.papertrailapp.com:10303                             |
|                               | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
+-------------------------------+--------------------------------------------------------------------+

@curzolapierre curzolapierre force-pushed the feature/571/list_log_drains_by_DB branch from 1714b2b to ae3ee93 Compare Jun 24, 2020
@curzolapierre curzolapierre requested a review from EtienneM Jun 24, 2020
Copy link
Member

@EtienneM EtienneM left a comment

Yes I think your manual implementation is a bit too heavy just for this. I wouldn't do that. But if I look at the 6th example of the doc of the tablewriter lib (https://github.com/olekukonko/tablewriter#example-6----identical-cells-merging), it looks like what we want, doesn't it?

@curzolapierre
Copy link
Contributor Author

curzolapierre commented Jun 24, 2020

The 6th example is what I implemented. It uses the combination of table.SetRowLine(true) with table.SetAutoMergeCells(true)
so in our case the input looks like:

+-------------------------------+--------------------------------------------------------------------+
|             NAME              |                                URL                                 |
+-------------------------------+--------------------------------------------------------------------+
| sample-go-martini             | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
+                               +--------------------------------------------------------------------+
|                               | tcp+tls://logs.papertrailapp.com:10303                             |
+-------------------------------+--------------------------------------------------------------------+
| Scalingo Redis                | ovh://:e8ea17bd-34e5-47b6-a016-675077053417@gra3.logs.ovh.com:6514 |
+                               +--------------------------------------------------------------------+
|                               | tcp+tls://logs.papertrailapp.com:10303                             |
+-------------------------------+--------------------------------------------------------------------+

@curzolapierre curzolapierre requested a review from EtienneM Jun 25, 2020
Copy link
Member

@EtienneM EtienneM left a comment

Just lack the go-scalingo release, otherwise, LGTM

Gopkg.toml Outdated Show resolved Hide resolved
@curzolapierre curzolapierre requested a review from EtienneM Jun 25, 2020
Copy link
Member

@EtienneM EtienneM left a comment

I forgot to ask you to add the changelog entry. Then LGTM

@curzolapierre curzolapierre requested a review from EtienneM Jun 25, 2020
@EtienneM EtienneM merged commit ffbbd75 into master Jun 25, 2020
1 check passed
@EtienneM EtienneM deleted the feature/571/list_log_drains_by_DB branch Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants