Select server config at runtime #963
Conversation
1. Rewrote in pytest 2. Removed reduntant tests 3. Added tests for add_user() and get_users()
mssalvatore
force-pushed
the
select-server-config-at-runtime
branch
2 times, most recently
from
de8ba4f
to
891700c
Compare
Codecov Report
@@ Coverage Diff @@
## appimage #963 +/- ##
============================================
+ Coverage 19.07% 19.10% +0.02%
============================================
Files 338 337 -1
Lines 11484 11487 +3
============================================
+ Hits 2191 2195 +4
+ Misses 9293 9292 -1
Continue to review full report at Codecov.
|
|
|
||
| def create_default_config_file(path): | ||
| default_config_path = f"{path}.default" |
There was a problem hiding this comment.
so now server_config.json.default file is no longer being used? Why I don't see it being deleted from the codebase?
There was a problem hiding this comment.
It's there for the convenience of developers. I can remove it or rename it to server_config.json.develop
There was a problem hiding this comment.
How is it convenient? The purpose of this file was to contain default value of server_config.json. We needed this whole process of config file deployment in order not to have server_config.json in our git, but have it on runtime.
There was a problem hiding this comment.
How is it convenient?
As a developer, doing cp server_config.json.default server_config.json in a new development environment is less error prone than editing the file by hand.
The purpose of this file was to contain default value of
server_config.json.
Default for whom? Naming it something like .develop or .standard is more useful than default.
We needed this whole process of config file deployment in order not to have
server_config.jsonin our git, but have it on runtime.
That functionality still works. It's just using server_config.json.standard by default if the specified server_config.json doesn't exist.
There was a problem hiding this comment.
As a developer, doing cp server_config.json.default server_config.json in a new development environment is less error prone than editing the file by hand.
How it worked before was that server_config.json was automatically created, which is even less error prone.
That functionality still works. It's just using server_config.json.standard by default if the specified server_config.json doesn't exist.
Then we're back at square one: why do we need server_config.json.default ? I think the downside of using standard config for development is that in our statistics we won't be able to distinguish our own monkey usage from client monkey usage, but that doesn't sound like a big problem. If we think this is a problem, we need our codebase to use develop config by default and change this behavior to "use standard instead" during release process. To phrase it differently, we want to use develop by default and change it to whatever as part of our release process.
The `get_from_json()` and `get_from_dict()` static methods were really just used for testing. The `EnvironmentConfig` class needs to store its file path so it can wite to the file if needed. In practical usage, `EnvironmentConfig` objects are initialized from files, so a simpler interface is for its constructor to take a file path.
mssalvatore
force-pushed
the
select-server-config-at-runtime
branch
from
891700c
to
52f80e8
Compare
mssalvatore
force-pushed
the
select-server-config-at-runtime
branch
from
e700e67
to
737d1bb
Compare
What does this PR do?
Allows the server config file to be selected at runtime, instead of hard coded in
EnvironmentConfig.Because AppImages are stored in a read_only squashfs, the server config file can not be stored within the source directory of Monkey Island.
PR Checklist
Testing Checklist