UCL-CloudLabs / user-portal

Front end user portal for CloudLabs
0 stars 0 forks source link

Improve naming #50

Closed ageorgou closed 6 years ago

ageorgou commented 6 years ago

A more structured attempt to make naming of resources, URLs etc smoother and consistent (this should replace #49). Mostly this moves some things around and separates the "base name" (given in the form) from the "DNS name" (the domain actually used, including random tokens to avoid clashes). Both are stored in the DB so they can be used later on (e.g. in assigning the UCL domain), and a new names module will contain relevant functions for generating names.

Note: This probably needs the migration code for adding a column to the DB schema; the default generated upgrade will fail if there are any rows already in the DB, as they will violate the non-null constraint. Editing the migration is easy, but I'm not sure what we're assuming for version-controlling the DB contents/migrations.

Other things to possibly implement (but maybe not in this PR?):

ageorgou commented 6 years ago

Tests seem to fail because they use syntax introduced in Python 3.5 and the tests run in 3.4. Sorry! I can change it - I assume we do want to test 3.4 too?

jonc125 commented 6 years ago

The generated (and possibly hand edited) migrations should definitely go in the repo. Each migration has a pair of methods - one for moving to the new structure, one for going back to the old one. And yes, if you need to avoid breaking a non-null constraint, you'll need to include custom code in the migration.

ageorgou commented 6 years ago

Thanks @jonc. Added my current naive workaround for the migration, which however does not display existing rows nicely in the dashboard. Presumably the best way would be to:

But I'm not sure how worried we are about how existing hosts are displayed?

jonc125 commented 6 years ago

Yeah, I don't like removing the uniqueness constraint as a workaround! That might lead SqlAlchemy to think it's there but the database not to have it, since the DB schema is created by applying the migrations sequentially.

Can you copy values from one column to another while creating? That might be a slightly quicker implementation in this case, especially since we don't have much data to migrate. But might be best just to do it properly :)

ageorgou commented 6 years ago

I don't think you can copy values while creating, hence the workaround, but I only had a relatively quick look. To be clear, the uniqueness constraint was only in the index anyway! (or so I think)

ageorgou commented 6 years ago

Migration still needs to be cleared up, ran into difficulties/confusion with sqlalchemy.

ageorgou commented 6 years ago

Test seem to fail because of a connection error? Edit: On second inspection, terraform fails to deploy the VM because of an error in the specification of the storage account. Perhaps an Azure change? Edit^2: Error seems to be because of #52.