Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add instructions to build docker image locally #262

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

amishas157
Copy link
Contributor

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into two PRs
    otherwise).
  • This PR's title starts with the jira ticket associated with the PR.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or fixes.
  • I've updated the README with the added features, breaking changes, new instructions on how to use the repository. I updated the description of the fuction with the changes that were made.

Release planning

  • I've decided if this PR requires a new major/minor/patch version accordingly to
    semver, and I've changed the name of the BRANCH to release/_ , feature/_ or patch/* .

What

This PR adds following changes:

  • Add instructions in README to build docker image locally and run export commands locally. This is helpful for local testing.

  • Update docker-build command in Makefile to support Apple M1 chip. See https://stackoverflow.com/questions/66662820/m1-docker-preview-and-keycloak-images-platform-linux-amd64-does-not-match-th

  • Update datastore path from sdf-ledger-close-metas/ledgers to sdf-ledger-close-meta/ledgers (Note the removed (s) in metas)
    Context from slack:

    "We recently did a full history backfill for our datalake in GCS in bucket sdf-ledger-close-meta/ledgers
    sdf-ledger-close-metas/ledgers is the old bucket. This was used as our frontfilling bucket but should be updated and changed in stellar-etl to the new full history data lake as the default bucket"

Why

Covered in what section

Known limitations

  • Check with @chowbao a good way to test above non-doc changes

@amishas157 amishas157 requested a review from a team as a code owner July 1, 2024 20:24
@@ -235,7 +235,7 @@ func AddCommonFlags(flags *pflag.FlagSet) {
flags.Bool("futurenet", false, "If set, will connect to Futurenet instead of Mainnet.")
flags.StringToStringP("extra-fields", "u", map[string]string{}, "Additional fields to append to output jsons. Used for appending metadata")
flags.Bool("captive-core", false, "If set, run captive core to retrieve data. Otherwise use TxMeta file datastore.")
flags.String("datastore-path", "sdf-ledger-close-metas/ledgers", "Datastore bucket path to read txmeta files from.")
Copy link
Contributor

Choose a reason for hiding this comment

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

🙌

@@ -7,7 +7,7 @@ BUILD_DATE := $(shell date -u +%FT%TZ)
ETLHASH=stellar/stellar-etl:$(shell git rev-parse --short HEAD)

docker-build:
$(SUDO) docker build --pull --no-cache --label org.opencontainers.image.created="$(BUILD_DATE)" \
$(SUDO) docker build --platform linux/amd64 --pull --no-cache --label org.opencontainers.image.created="$(BUILD_DATE)" \
Copy link
Contributor

@chowbao chowbao Jul 1, 2024

Choose a reason for hiding this comment

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

I recommend not adding --platform to the Makefile as it is used in our https://github.com/stellar/pipelines/blob/master/stellar-etl/Jenkinsfile#L44-L45

Edit: NVM it should be fine to add as it'd be a noop. For more context: we all have macbooks with m* chips so we need to force the x86 (e.g. linux/amd64) build

@amishas157 amishas157 merged commit b3fb467 into master Jul 2, 2024
7 checks passed
@sydneynotthecity sydneynotthecity deleted the docs/add-local-setup-instructions branch July 15, 2024 13:02
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants