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.

Sign up for GitHub

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

Jump to bottom

PLT-5847 marlowe-contract service #578

Merged
merged 23 commits into from May 24, 2023
Merged

PLT-5847 marlowe-contract service #578

merged 23 commits into from May 24, 2023

Conversation

jhbertra
Copy link
Contributor

@jhbertra jhbertra commented May 10, 2023 •
edited

  • Adds new marlowe-contract service to runtime topology
  • Adds MarloweLoad protocol to MarloweRuntime protocol
  • Adds load command to marlowe-runtime-cli
  • Updates docker-compose, operable and OCI images
  • TODO Updates nomad tasks
  • TODO adds documentation for JSON format accepted by load command.

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Formatting, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
      • Review required
      • Substantial changes to code, test, or documentation
    • Reviewer requested

@jhbertra jhbertra changed the title Plt 5847 marlowe loader PLT-5847 marlowe-contract service May 11, 2023
@jhbertra jhbertra force-pushed the plt-5847-marlowe-loader branch 2 times, most recently from b41ffa3 to 1402a68 Compare May 15, 2023 19:21
@jhbertra jhbertra changed the base branch from main to jhbertra/refactor-marlowe-proxy May 15, 2023 19:44
@jhbertra jhbertra force-pushed the jhbertra/refactor-marlowe-proxy branch 2 times, most recently from d9fbdc0 to 15c446e Compare May 16, 2023 21:23
@jhbertra jhbertra force-pushed the jhbertra/refactor-marlowe-proxy branch from 15c446e to 0b1da8a Compare May 17, 2023 14:28
Base automatically changed from jhbertra/refactor-marlowe-proxy to main May 17, 2023 14:44
@jhbertra jhbertra requested a review from bwbush May 17, 2023 17:22
@jhbertra jhbertra self-assigned this May 17, 2023
@jhbertra jhbertra force-pushed the plt-5847-marlowe-loader branch 2 times, most recently from d623136 to 1cb6ab3 Compare May 17, 2023 18:40
@jhbertra jhbertra marked this pull request as ready for review May 17, 2023 18:50
bwbush
bwbush reviewed May 19, 2023
View reviewed changes
deploy/operables.nix Outdated Show resolved Hide resolved
bwbush
bwbush reviewed May 19, 2023
View reviewed changes
marlowe-runtime-cli/app/Language/Marlowe/Runtime/CLI/Command/Load.hs
countNodes :: Int -> FilePath -> CountM Int
countNodes count path = withContract path $ countNodes' count

countNodes' :: Int -> LoadContract -> CountM Int
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if the multiplate in Language.Marlowe.Core.V1.Plate could be used to do this counting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly, the concept of a Node is somewhat abstract though, and isn't directly captured by anything in there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, I see that LoadContract is not Contract, which is what the plate acts on. ☹️

bwbush
bwbush reviewed May 19, 2023
View reviewed changes
marlowe-runtime/contract/Language/Marlowe/Runtime/Contract/Store/File.hs
import System.IO.LockFile (withLockFile)
import UnliftIO
import UnliftIO.Directory
( XdgDirectory(XdgData)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the XDG stuff portable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is

bwbush
bwbush reviewed May 19, 2023
View reviewed changes
marlowe-runtime/marlowe-contract/Main.hs Outdated
bufferSizeParser = option readOption $ mconcat
[ long "buffer-size"
, short 'b'
, value 4098
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems too high. Are exceptions caught in a way the forces a flush before exit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1024 could be a better default. The staging area is wrapped in a bracket that discards the entire temp staging directory on exception. So there should be no resource leak (though nothing will protect from a SIGKILL or a loss of power of course).

bwbush
bwbush approved these changes May 19, 2023
View reviewed changes
Copy link
Collaborator

@bwbush bwbush left a comment

Choose a reason for hiding this comment

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

Looks great. I'll do an on-chain test when marlowe-runtime-cli create supports this.

@jhbertra jhbertra force-pushed the plt-5847-marlowe-loader branch 2 times, most recently from 0c1480d to 2b7f9a1 Compare May 24, 2023 15:27
@jhbertra jhbertra enabled auto-merge May 24, 2023 15:29
jhbertra and others added 19 commits May 24, 2023 13:36
@jhbertra
Add marlowe-contract executable
d091f37
@jhbertra
Add MarloweLoad to MarloweRuntime protocol
99a69ff
@jhbertra
Add load command to cli
0215d72
@jhbertra
fix marlowe-integration
b914df9
@jhbertra
Run contract in integration test container
6b2ce79
@jhbertra
Document JSON format for load
1b97ad7
@jhbertra
Refactor load to avoid chainging directory for extended periods
12780cd
@jhbertra
Fix load path resolution and marlowe-proxy tracing
bccfbdf
@jhbertra
Run sqitch in runtime scripts for docker compose
edb4d7a
@jhbertra
Fix test build
5f32ffa
@jhbertra
Add marlowe-contract nomad task
1f30d8d
@jhbertra
Remove ContractSpec tests from marlowe-runtime-test executable
a453f10
@jhbertra
Add binary instances for all core types
4a951f8
@jhbertra
Save contract directly without converting to datum
56b3aae
@jhbertra
Create staging area in correct directory
df2245a
@jhbertra
Use direct binary instances in load protocol
25379b8
@jhbertra
Update changelog
93a81a8
@jhbertra @bwbush
Update deploy/operables.nix
6ae8400 
Co-authored-by: Brian W Bush <brian.bush@iohk.io>
@jhbertra
Reduce size of default load buffer
a2d7ff7
@jhbertra jhbertra merged commit c60bf21 into main May 24, 2023
36 checks passed
@jhbertra jhbertra deleted the plt-5847-marlowe-loader branch May 24, 2023 17:50
@Sammyreal1 Sammyreal1 linked an issue Jun 8, 2023 that may be closed by this pull request
Approved #604
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwbush bwbush bwbush approved these changes

Assignees

@jhbertra jhbertra

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Approved
2 participants
@jhbertra @bwbush