-
Notifications
You must be signed in to change notification settings - Fork 50
[WIP] Remove MongoDB dependency from core tests #582
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
6b6669d
Remove MongoDB dependencies from tests/functional/commands
breuleux 0bb0c5e
Remove MongoDB dependencies from tests/functional/branching
breuleux 9ba9e7a
Remove MongoDB dependencies from tests/functional/parsing
breuleux 1fa5341
Remove MongoDB dependencies from tests/functional/demo
breuleux 5b2970f
Remove MongoDB dependencies from tests/unittest/core (partial)
breuleux 3d8a3b5
Fix formatting
breuleux a566afe
Remove MongoDB dependencies from tests/functional/client
breuleux 2c2ae14
Remove MongoDB dependencies from tests/unittests/core/worker/test_pro…
breuleux 385164b
Remove MongoDB dependencies from tests/unittests/core/io
breuleux a021142
Fix parent_id in child_config fixture
breuleux 2720be0
Remove create_db_instance fixture
breuleux e1777b9
Remove database from yaml configs
breuleux 97aa169
Change default storage to PickledDB
breuleux 569dfd7
Remove MongoDB dependencies
abergeron 93ed65d
Revert "Change default storage to PickledDB"
breuleux 699012c
Revert "Remove database from yaml configs"
breuleux 67ca3c7
Use storage instead of pdatabase in evc
breuleux 6ad9a1c
Nuke test_experiment_tree
breuleux 10e0126
Remove pdatabase
breuleux 055e1e6
Remove more uses of _fetch_trials
breuleux 40c3b00
Remove useless usefixtures
breuleux f1bceb0
Merge branch 'develop' into rm_mongo_dep
breuleux 9b35f07
Avoid OrionState in hacked_exp fixture to fix storage fixture issue
breuleux 10243f4
Fix database_test::TestReadOnlyDatabase.test_read
breuleux cbf9e86
Re-trigger tests
breuleux 0f81bd9
Fix bug in update_heartbeat
breuleux File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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.
You will need to change the host below as well. You should set it to default host for pickleddb: https://github.com/Epistimio/orion/blob/develop/src/orion/core/io/database/pickleddb.py#L25
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.
Ah, right. Shouldn't the default here depend on the actual value of the
type
option? I mean, iftype
is set tomongodb
thenhost
should belocalhost
by default, not PickledDB'sDEFAULT_HOST
.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.
Ideally yes, but
Configuration
does not support conditional defaults like this for the moment. :(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.
The default could be None or "", though, and then the storage's constructor can set it to its default if it sees the host argument is null.
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.
Good idea!