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

Fix migrations #4229

Merged
merged 3 commits into from Jan 8, 2019
Merged

Fix migrations #4229

merged 3 commits into from Jan 8, 2019

Conversation

@lasote
Copy link
Contributor

@lasote lasote commented Jan 4, 2019

Changelog: Bugfix: The migrated data in the server from a version previous to Conan 1.10.0 was not migrated creating the needed indexes. This fixes the migration and creates the index on the fly for fixing broken migrations. Also the server doesn't try to migrate while running but warns the user to run conan server --migrate after doing a backup of the data, avoiding issues when running the production servers like gunicorn where the process doesn't accept input from the user.

Docs: omit
Closes #4219

@ghost ghost assigned lasote Jan 4, 2019
@ghost ghost added the stage: review label Jan 4, 2019
@lasote lasote added this to the 1.11.2 milestone Jan 4, 2019
@lasote lasote requested review from memsharded and jgsogo Jan 4, 2019
Version(SERVER_VERSION), Version(MIN_CLIENT_COMPATIBLE_VERSION),
server_capabilities)

print("***********************")
Copy link
Contributor

@SSE4 SSE4 Jan 7, 2019

why not conan output?

Copy link
Contributor Author

@lasote lasote Jan 8, 2019

The migrations and the server launcher are run "pre-conan". So consider them as scripts, we could convert them but I think it is ok to keep them as lean as possible.

server_capabilities = SERVER_CAPABILITIES
server_capabilities.append(REVISIONS)

self.ra = ConanServer(server_config.port, credentials_manager, updown_auth_manager,
Copy link
Contributor

@SSE4 SSE4 Jan 7, 2019

what does "RA" stand for? may be rename?

server_config.public_url,
updown_auth_manager=updown_auth_manager)

server_capabilities = SERVER_CAPABILITIES
Copy link
Contributor

@SSE4 SSE4 Jan 7, 2019

one line server_capabilities = [SERVER_CAPABILITIES, REVISIONS]

Copy link
Contributor Author

@lasote lasote Jan 8, 2019

That would create a list with a first argument being a list.

Copy link
Member

@memsharded memsharded Jan 8, 2019

But as currently defined in conan:

SERVER_CAPABILITIES = [COMPLEX_SEARCH_CAPABILITY, REVISIONS]

Copy link
Member

@memsharded memsharded left a comment

I'd probably decouple migrations from launch, i.e. conan_server --migrate only migrates, but does not launch the server.

from conans.server.service.authorize import BasicAuthorizer, BasicAuthenticator


class ServerLauncher(object):
Copy link
Member

@memsharded memsharded Jan 7, 2019

The rename of the file doesn't help to review :S

Copy link
Contributor Author

@lasote lasote Jan 8, 2019

The gunicorn launching of the server is documented and cannot be broken, so the module name cannot be changed. At the same time, crossed imports blocked the possibility of init the application with the migration flag so that change was mandatory in order to implement the fix, sorry.

@lasote
Copy link
Contributor Author

@lasote lasote commented Jan 8, 2019

Done, the "ra" has been renamed and the server with --migrations doesn't run the server.

@lasote lasote merged commit 20f6f48 into conan-io:release/1.11.2 Jan 8, 2019
2 checks passed
@ghost ghost removed the stage: review label Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants