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

feat: remote state backends #6911

Merged
merged 6 commits into from Nov 11, 2022
Merged

feat: remote state backends #6911

merged 6 commits into from Nov 11, 2022

Conversation

cjohnhanson
Copy link
Contributor

@cjohnhanson cjohnhanson commented Oct 18, 2022

@netlify
Copy link

netlify bot commented Oct 18, 2022

Deploy Preview for meltano ready!

Name Link
🔨 Latest commit 6b581e5
🔍 Latest deploy log https://app.netlify.com/sites/meltano/deploys/636e6a445c341800087972a6
😎 Deploy Preview https://deploy-preview-6911--meltano.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@WillDaSilva WillDaSilva left a comment

Choose a reason for hiding this comment

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

I have no major concerns with what's here. Really glad to see this coming along nicely!

docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
schema/meltano.schema.json Outdated Show resolved Hide resolved
schema/meltano.schema.json Outdated Show resolved Hide resolved
@aaronsteers
Copy link
Contributor

From 2022-11-03 discussion:

  1. Environment-level support todo:
    • Cody to check if this works when set the environment level.
    • Undo env_specific: True and instead use kind: password
    • Here or elsewhere: add back full support for meltano settings that are allowed to be environment specific.
  2. Integration tests

@cjohnhanson cjohnhanson marked this pull request as ready for review November 4, 2022 21:31
@cjohnhanson cjohnhanson requested review from WillDaSilva and edgarrmondragon and removed request for pandemicsyn November 8, 2022 21:23
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #6911 (79e6b8a) into main (be573b3) will decrease coverage by 0.06%.
The diff coverage is 87.88%.

❗ Current head 79e6b8a differs from pull request most recent head 6b581e5. Consider uploading reports for the commit 6b581e5 to get more accurate results

@@            Coverage Diff             @@
##             main    #6911      +/-   ##
==========================================
- Coverage   88.76%   88.70%   -0.07%     
==========================================
  Files         284      292       +8     
  Lines       20573    21281     +708     
  Branches     2028     2104      +76     
==========================================
+ Hits        18262    18877     +615     
- Misses       1976     2051      +75     
- Partials      335      353      +18     
Impacted Files Coverage Δ
tests/meltano/cli/test_run.py 98.38% <ø> (ø)
src/meltano/core/plugin/singer/target.py 81.96% <50.00%> (ø)
src/meltano/core/state_store/filesystem.py 74.71% <74.71%> (ø)
src/meltano/core/state_store/s3.py 75.55% <75.55%> (ø)
src/meltano/core/state_store/google.py 76.08% <76.08%> (ø)
src/meltano/core/state_store/azure.py 79.54% <79.54%> (ø)
src/meltano/core/job_state.py 89.47% <85.18%> (-4.47%) ⬇️
src/meltano/core/block/extract_load.py 84.83% <90.00%> (+0.09%) ⬆️
src/meltano/core/state_store/__init__.py 92.85% <92.85%> (ø)
tests/meltano/core/state_store/test_filesystem.py 95.03% <95.03%> (ø)
... and 21 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

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

Some docs suggestions now that #6628 has been merged

docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
docs/src/_concepts/state_backends.md Outdated Show resolved Hide resolved
cjohnhanson and others added 3 commits November 9, 2022 07:53
Co-authored-by: Edgar R. M. <edgarrm358@gmail.com>
Co-authored-by: Edgar R. M. <edgarrm358@gmail.com>
Co-authored-by: Edgar R. M. <edgarrm358@gmail.com>
@cjohnhanson cjohnhanson merged commit a0aa2ec into main Nov 11, 2022
@cjohnhanson cjohnhanson deleted the 5981-state-backends branch November 11, 2022 15:45
@pdebelak
Copy link

👋 - question about this. I'm currently blocked on upgrading meltano from version 2.1.0 due to using Ubuntu 18.04 with an old version of sqlite so I thought that setting a different state backend would allow me to not care about the sqlite version. It appears that setting the state backend uri to a file location doesn't stop meltano from running migrations on its sqlite database anyway. Is that expected?

@edgarrmondragon
Copy link
Collaborator

@pdebelak yes, that's unfortunately expected at this point. I wonder if now that Meltano supports secrets backends, Meltano should allow users to disable database migrations, or disable the backend db entirely.

cc @aaronsteers @tayloramurphy

@aaronsteers
Copy link
Contributor

aaronsteers commented Nov 30, 2022

@edgarrmondragon, @pdebelak - We have a bit more to do before we can fully replace systemdb.

  • State Backends
  • Job collision prevention (locking handled
    via state backend implementation)
  • Settings+Secrets Backends
  • Logging Backend

However, since logging is "write only" as of now, perhaps we could disable the backend, in theory at least. If so, we'd have to fail if config get/set is attempted against systemdb, and we'd skip consideration of systemdb when using the auto-store decision tree for getting/setting config values.

It would probably take a lot of testing and debugging to get right, but in theory it may be possible... 🤔

@aaronsteers
Copy link
Contributor

@edgarrmondragon, @pdebelak - We have a bit more to do before we can fully replace systemdb.

  • State Backends
  • Job collision prevention (locking handled
    via state backend implementation)
  • Settings+Secrets Backends
  • Logging Backend

However, since logging is "write only" as of now, perhaps we could disable the backend, in theory at least. If so, we'd have to fail if config get/set is performed against it, and we'd skip consideration of systemdb when using the auto-store decision tree for getting/setting config values.

It would probably take a lot of testing and debugging to get right, but in theory it may be possible... 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Discussed
Development

Successfully merging this pull request may close these issues.

feat: Add support for non-DB state backends (s3, dynamodb, etc.)
5 participants