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
Platform integration: Download all databases, Allow a primary relationship for the main 'db', fixes #4415, fixes platformsh/ddev-platformsh#59 #4426
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
9d7ac4a
Allow for primary relationship in platform.sh pull/push
rfay f3f4ca0
on ddev pull, allow importing a set of databases, not just one
rfay d6580c9
Sort out difference between relationship and db name
rfay f0fae87
Make files import work
rfay f122105
Test*Pull/Push working
rfay 6feb45b
Handle PLATFORM_PRIMARY_RELATIONSHIP again
rfay 48b1abd
Update docs
rfay 1e1eefa
Pacify linter
rfay 18d3600
Fix type in failure [skip ci]
rfay File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rfay I understand only one database will be handled by the integration, but what would you recommend if I had additional databases I also wanted to pull into DDEV?
Create my own modified version of
platform.yaml
and modifydb_pull_command
?I ask because I’m looking at the docs, and designating a primary naturally begs the question of what I should do with my other ones. Even if it’s just a hint, it seems like it’d be worth addressing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this version, all databases will be handled by the
ddev pull platform
, so line 45 is just incorrect. Thanks for catching that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I understanding correctly that all databases will be pulled, but that I still need to designate a
PLATFORM_PRIMARY_RELATIONSHIP
so DDEV knows what to use for its (default)'db'
database?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ultimately asking while I attempt to improve the wording over here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattstein this is correct.
db
database of DDEV (the default one)I believe I've never used the word database that much in such a short message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bserem Thanks for clarifying the database behavior for this database-related documentation adjustment regarding databases. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all projects will need for a "db" database to be populated. For example, as the ddev-platformsh add-on matures, it should be able to handle databases of any number that have the same names they do on Platform.sh, and the "db" database not be used at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is more of an if you have multiple databases and want to designate a primary to use with
'db'
, yes? If so I’ll update #4469 accordingly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is correct. Most people who have ever used DDEV will want that. There may be contexts in the future where that's not needed. The same may be true for Acquia or Pantheon or other hosts, where the settings are wired to particular database names and people are not using DDEV's automatically generated settings, like
settings.ddev.php
or Craft CMS's .env configuration.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revised in this commit, which is hopefully more accurate! 😅