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

Outputs and push #52

Open
wants to merge 11 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@roberth
Copy link
Contributor

commented May 7, 2019

  • configurable push caches
  • push implementation
  • push event
  • output info events
  • integration test for build task + output info
  • clean up the WIP commit (Pushing -> Pushed)
  • cachix push testing
  • push drvs

@roberth roberth requested a review from domenkozar May 7, 2019

@domenkozar
Copy link
Member

left a comment

Mostly minor things :)

Show resolved Hide resolved hercules-ci-agent/hercules-ci-agent/Hercules/Agent/Build.hs Outdated
Show resolved Hide resolved hercules-ci-agent/hercules-ci-agent/Hercules/Agent/Build.hs Outdated
Show resolved Hide resolved hercules-ci-agent/hercules-ci-agent/Hercules/Agent/Build.hs Outdated
Show resolved Hide resolved hercules-ci-agent/hercules-ci-agent/Hercules/Agent/Build.hs Outdated
getAvailablePushCaches = do
env <- asks (cachixEnv . getEnv)
let
pushableName Cachix.Config.BinaryCacheConfig { secretKey = _required, name = name }

This comment has been minimized.

Copy link
@domenkozar

domenkozar May 7, 2019

Member

I guess this is not done yet? :)

This comment has been minimized.

Copy link
@roberth

roberth May 7, 2019

Author Contributor

_required is mostly there for the compiler to notify if the field is removed, or renamed, because that might indicate a problem. I guess I should add a type signature to make that work, but the real solution is to improve the cachix library so it exposes a function that does exactly what this function getAvailablePushCaches does :)

roberth added some commits May 9, 2019

@domenkozar domenkozar referenced this pull request May 10, 2019

Open

Darwin support? #31

@roberth roberth marked this pull request as ready for review May 10, 2019

@roberth roberth requested a review from domenkozar May 10, 2019

@domenkozar
Copy link
Member

left a comment

Only one behavior change that I think we should address.

Otherwise this change should come with accompanying documentation for cachix+agent setup via nixops that will then be linked from the frontend.

@@ -142,7 +146,7 @@ executable hercules-ci-agent-worker
ghc-options: -Wall -fwarn-tabs -fwarn-unused-imports -fwarn-missing-signatures -fwarn-name-shadowing -fwarn-incomplete-patterns -threaded -rtsopts -with-rtsopts=-N
-- GC_INCLUDE_NEW to use <new> header and standard C++ exception on OOM
cc-options: -Wall -std=c++11
ld-options: -lstdc++
extra-libraries: stdc++

This comment has been minimized.

Copy link
@domenkozar

domenkozar May 13, 2019

Member

Needs a rebase :)

activePushCaches :: (MonadReader r m, HasEnv r) => m [Text]
activePushCaches = asks (pushCaches . getEnv)

validate :: (MonadReader r m, HasEnv r, Katip m) => m ()

This comment has been minimized.

Copy link
@domenkozar

domenkozar May 13, 2019

Member

It's documented and public interface to use CACHIX_SIGNING_KEY=... cachix push foo without having a configuration file. The reasoning is that the key might have been generated on another machine.

validate will mislead the user that cache won't work, although it will.

I don't think we have a choice but to support CACHIX_SIGNING_KEY= as there's no way to write cachix configuration file given the signing key atm.

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.