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

sheldon test #3

Open
wants to merge 22 commits into
base: reporting
from

Conversation

Projects
None yet
3 participants
@adam3smith
Copy link

commented May 30, 2019

No description provided.

@csl-bot

This comment has been minimized.

Copy link

commented May 30, 2019

😟 There are some issues with your submission. Please check the test report for details.

@retorquere

This comment has been minimized.

Copy link
Owner

commented May 30, 2019

I'm not sure why Sheldon isn't picking this up. I'm looking into it.

@retorquere

This comment has been minimized.

Copy link
Owner

commented May 30, 2019

Can you merge the latest reporting branch into this PR and push the changes? Travis runs on a old bundler which doesn't support git-installable gems. I've added a bundler update to the travis config.

@retorquere

This comment has been minimized.

Copy link
Owner

commented May 30, 2019

Oh wait -- the github token. Hold on.

retorquere added some commits May 30, 2019

@retorquere

This comment has been minimized.

Copy link
Owner

commented May 30, 2019

The github token has been set up, and the travis config updates bundler before sheldon runs now -- can you merge latest from this branch and re-push?

@retorquere

This comment has been minimized.

Copy link
Owner

commented May 31, 2019

And that is the one thing I had not considered. For security reasons, encrypted variables (such as authentication tokens) are not set in PRs for security reasons (otherwise a malicious actor could just echo them in the PR and steal the token)

Damn.

I'm truly sorry for all this noise BTW -- can I submit PRs myself on my own repo so I can test myself? Or can I push changes onto this PR myself?

I'll have to fall back to the other option I had in mind -- publishing the test assets somewhere like transfer.sh and having sheldon pick them up. I'll look into this.

@adam3smith

This comment has been minimized.

Copy link
Author

commented May 31, 2019

I'm truly sorry for all this noise BTW -- can I submit PRs myself on my own repo so I can test myself? Or can I push changes onto this PR myself?

You can create a different branch and make PRs between them (and no worries -- really appreciate all your work on this)

@adam3smith

This comment has been minimized.

Copy link
Author

commented Jun 4, 2019

OK, let's see if that works. Also happy to just open a new PR

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 4, 2019

We'll need a new PR to verify the welcome still works, but the actual thing that's changed to have sheldon pick up the details should work here too. Did you merge the reporting branch? Your .travis.yml should have an invocation of bundle exec sheldon --verbose, and I don't see the --verbose in the travis log.

@adam3smith

This comment has been minimized.

Copy link
Author

commented Jun 4, 2019

I did merge reporting, but not seeing bundle exec sheldon --verbose in .travis.yml in your reporting branch?
https://github.com/retorquere/styles/blob/reporting/.travis.yml

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 4, 2019

Forgot to commit 🙄

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 4, 2019

It's in the reporting branch now.

retorquere added some commits Jun 4, 2019

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 4, 2019

never mind -- I can see the hidden message is successfully put in the log. The question is now why it's not being picked up by my instance of shel-bot. On it.

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 4, 2019

I had misconfigured the webhook URL. Can you try again?

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 5, 2019

I think we're getting there -- there's something really strange going on with owner-submitted PRs, if you look at https://travis-ci.org/retorquere/styles/builds/541564390 you'll see that travis builds this on "master" rather than "junk", and Shel-gem bails if building on master (as there would be nothing to compare to for styles). I've submitted a support request to Travis for this, but it means effectively I cannot test using my own PRs.

retorquere added some commits Jun 5, 2019

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 6, 2019

Getting the log from shel-bot seems to get a mutilated version of the log. When I fetch the log later, it is fine. Maybe a race condition - im in touch with Travis support again. The way out would be to post the snippet to a well-know url, which gets us back to a anon-write to an b2 or s3 bucket. How do you guys feel about this.

I'd never have thought that just getting the snippet to shel-bot would be the bulk of the work!

retorquere added some commits Jun 7, 2019

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2019

finally got it to work. In the end I've added a memcache addon to sheldon. It doesn't really matter what's used to store the build details as long as shel-bot has read/write access to it, so I'm open to suggestions. Hiding in the log worked in the end, but the log display of Travis would never consistently interpret the hiding ANSI codes so you'd always see some of the details leak out to the display.

Can you try merging the reporting branch again? It now also works for my own PRs.

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2019

memcache has the benefit that it'll auto-expire the snippets to keep size down (given that they so far have been below 25KB, the 30MB free allotment should go a long way), but S3 also has auto-expiring of assets, so that should also work and be low-maintenance. Heroku Pg could also be made to work.

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2019

😟 There are some issues with your submission.

1 test failed

acta-naturae: must validate against the CSL 1.0.1 schema
Please check your style at http://validator.citationstyles.org/

expected `[[24, "24:0: ERROR: Did not expect element macro there"], [24, "24:0: ERROR: Element style has extra content: macro"], [2, "2:0: ERROR: Expecting element locale, got style"]].empty?` to return true, got false

Please check the test report for details.

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2019

Alright, that works! So then we need to talk about the persistence method for Sheldon. Here's some of the possibilities I see offhand:

  • We can keep memcached. This is in all likelyhood the most stable option, but the free heroku add-on states

We’ll reclaim the free resource after 30 days of being inactive. Limited to one free bucket per Heroku account.

  • Use transfer.sh. I've had problems with this in the past and also recently; the service occasionally seems to time-out when posting assets, and even when it's not, it's pretty slow (regularly taking 10 secs to upload 10K)
  • 0x0.st doesn't seem to suffer like transfer.sh but it explicitly states "no spamming build assets", and the service owner is fairly combative in his communications. I don't see that as promising for him widening existing use restrictions
  • We could (ab)use pastebin
  • S3 would work, and coupled with a lifecycle policy should keep costs minimal
  • B2 might be even cheaper but their lifecycle policy hangs on to objects longer
  • Heroku Pg can host 10k snippets on the free plan, and snippets are short-lived, so that could work too
@adam3smith

This comment has been minimized.

Copy link
Author

commented Jun 7, 2019

Hooray indeed!
memcached sounds good. I'm not worried about the 30 days -- it's not like we're every going to be idle for that long ever. (thanks for doing all the recon on the others; S3 would be my next preference).

Can you prep this as PRs to our repos?

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2019

Will do.

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2019

I'd hugely appreciate it if I could submit the PRs as-is and that github squash-and-merge is used to mash everything together. After all this time, rebase still terrifies me.

By necessity I will be submitting 3 PRs, to Sheldon, styles and locales. Here's how I recommend doing the rollout:

  1. Set up the memcached addon. heroku addons:create memcachedcloud should do the trick.
  2. Inspect and merge Sheldon. This will roll out the new Shel-bot (I think, there's a deploy stanza in .travis.yml)

At this stage, Sheldon should be ready to serve the new-style requests, but since it s backwards compatible, it won't do anything yet.

  1. Inspect and merge styles and locales. Order should not matter.

Agreed? I'll precede the PR names with "Sheldon build details:" to make them easily recognizable.

@adam3smith

This comment has been minimized.

Copy link
Author

commented Jun 7, 2019

I'd hugely appreciate it if I could submit the PRs as-is and that github squash-and-merge is used to mash everything together. After all this time, rebase still terrifies me.

Absolutely -- Dan at Zotero is much pickier about this than we are. (brag: I did the remote branch pull and rebase for this without googling/manual. I feel like I've graduated Defense against the Dark Arts II )

Yes, order for PRs sounds exactly right. Let me know if you need anything from us in the process (other than reviewing & merging PRs, or course).

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2019

It depends on how much effort you want post-merge. I can prep the styles and locales PRs to require no changes afterwards, but for that the Sheldon PR needs to be merged to master first, as the styles/locales repos need to say which branch to fetch shel-gem from; ideally, this would be master from https://github.com/citation-style-language/Sheldon.git, but there's no gem present there currently of course.

@retorquere

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2019

Absolutely -- Dan at Zotero is much pickier about this than we are.

I think Dan has allowed me to default to squash-and-merge before. I do understand why he wouldn't want the full messy commit trail

(brag: I did the remote branch pull and rebase for this without googling/manual. I feel like I've graduated Defense against the Dark Arts II )

I stand in awe. For me, this process vies for status as apex-stressor with doing a PhD.

@adam3smith

This comment has been minimized.

Copy link
Author

commented Jun 7, 2019

I can prep the styles and locales PRs to require no changes afterwards, but for that the Sheldon PR needs to be merged to master first

Yes, let's do that. I think we should get the Sheldon update merged quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.