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

#2389 [database:log:poll] Add a command to poll the db logging for dev environments #2529

Closed
wants to merge 7 commits into from

Conversation

twhiston
Copy link
Contributor

Command for above feature request

@enzolutions
Copy link
Contributor

Hi @twhiston

Thanks for your contribution, but looks like is duplicate with command database:log:debug, please confirm.

@twhiston
Copy link
Contributor Author

@enzolutions database:log:debug does not actively poll the database output and return any new lines every x seconds, it just prints the messages that are in the log currently. The poll command was added precisely to address this shortcoming in the debug command

@enzolutions
Copy link
Contributor

Uhhh could you show me a screenshot of the command in action?

Do you think is possible to create a trait to reuse some code for database:log:* commands?

@twhiston
Copy link
Contributor Author

I sure can

screen shot 2016-07-19 at 16 01 55

but it makes the most sense if you run it. Basically it keeps running, and every x seconds (set by user input or defaulting to 30s) it checks the db again to see if any new log messages are in the db and if there are it prints them to the console. As you can see the first message was printed when the command was started, the 2 errors after are printed after the default 30 seconds, because i did something on my site that caused a logging event to occur

Its absolutely possible to create a trait and use that across all the database:log:* commands. I wanted to submit this command on its own before considering any major refactoring of other commands as I dont know how you guys feel, as a group, about commands that poll and arnt just 'run and done'

@enzolutions
Copy link
Contributor

Don't do the Trait yet, let me review the command with @jmolivas, we are traveling today, give us couple days

@twhiston
Copy link
Contributor Author

no problem at all! If you need any more info about anything to do with the command i'm happy to discuss it with you and if you guys accept it i'll happily refactor all the database commands for better code reuse

@jmolivas jmolivas modified the milestone: 1.0.0-rc1 Jul 27, 2016
@enzolutions
Copy link
Contributor

@twhiston after talking with @jmolivas, we agree to include this command in next RC1, but first, you need to refactor the code to create a trait. Let me know if you are OK with that.

@twhiston
Copy link
Contributor Author

twhiston commented Aug 2, 2016

sounds like a plan. Shall i refactor the other database:log:* commands to use the trait as well?

@enzolutions
Copy link
Contributor

Correct, if you can refactor the other command too, would be a perfect PR for us

@twhiston
Copy link
Contributor Author

twhiston commented Aug 2, 2016

will do! What's the deadline for rc1? A bit busy at work the next week but i'd like to get all my PR's in before the deadline

@jmolivas
Copy link
Member

jmolivas commented Aug 2, 2016

@twhiston The ETA for RC-1 is Monday the 8th.

@jmolivas jmolivas modified the milestones: 1.0.0-rc2, 1.0.0-rc1 Aug 7, 2016
@twhiston
Copy link
Contributor Author

can you let me know if how i have started to implement this trait is how you want. I wasn't exactly sure what the proper way to manage the dependencies was now, as the current dev version of Console seems to allow it, but the one we are using on our sites doesn't have any injection at all. What is the official line on this? If you can point me in the right direction I will get this finished

@jmolivas
Copy link
Member

@twhiston I will take a look a this PR

@jmolivas jmolivas modified the milestones: 1.0.0-rc2, 1.0.0-rc3 Oct 3, 2016
@jmolivas
Copy link
Member

jmolivas commented Oct 8, 2016

@twhiston sorry for the late reply, but tagging the RC took longer that expected do you mind to sync your PR to have it merged.

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

Successfully merging this pull request may close these issues.

3 participants