Skip to content

Conversation

@ksaric
Copy link
Contributor

@ksaric ksaric commented Jul 4, 2018

https://iohk.myjetbrains.com/youtrack/issue/TSD-47

Run it for almost a day and it skips "phantom" tickets and inserts the data into the database.
After it's in the database, the data is available almost instantly.
There is still plenty of work to be done, one of the priorities is to add exception handling because it's getting very painful to run something just to see it crash a couple of minutes after that.

@ksaric ksaric requested review from HirotoShioi and robcohen July 4, 2018 07:44
@ksaric ksaric added the enhancement New feature or request label Jul 4, 2018
Copy link
Contributor

@HirotoShioi HirotoShioi left a comment

Choose a reason for hiding this comment

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

Nice work! Just few changes. I'll take a further look afterwards to see if we can make more improvements.

, cfgZendesk = "https://iohk.zendesk.com"
, cfgToken = ""
, cfgEmail = "daedalus-bug-reports@iohk.io"
, cfgEmail = "kristijan.saric@iohk.io"
Copy link
Contributor

Choose a reason for hiding this comment

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

You should revert this to daedalus-bug-reports@iohk.io. This address is used when the classifier is posting comments on Zendesk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to move some of these to CLI, this doesn't make sense here.

:: forall m. (MonadIO m, MonadReader Config m)
=> ExportFromTime
-> m [TicketInfo]
getExportedTickets time = do
Copy link
Contributor

@HirotoShioi HirotoShioi Jul 5, 2018

Choose a reason for hiding this comment

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

This function seems to be doing a lot of unnecessary printing.. Can you clean it up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't say it's "unnecessary" since I required that in order to run it and find three major issues.
I have an alternative though, will do it in a separate ticket.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

let typedURL = showURL $ TicketCommentsURL ticketId
untypedURL = "/tickets/" <> show (getTicketId ticketId) <> "/comments.json"
in typedURL == untypedURL
it "returns valid ExportDataByTimestamp" $ do
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is failing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Speaking of failing tests, did you add these:

  •  processes ticket, with comments FAILED [1]
    
  •  processes ticket, with no attachments FAILED [2]
    
  •  processes ticket, with attachments FAILED [3]
    

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I did, and it's strange because those tests are passing in the develop branch.

Copy link
Contributor

@HirotoShioi HirotoShioi left a comment

Choose a reason for hiding this comment

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

Besides failing tests, everything seems good!

@ksaric ksaric merged commit 6862dc2 into develop Jul 5, 2018
@ksaric ksaric deleted the ksaric/TSD-47 branch July 5, 2018 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants