[ENG-6650][ENG-6729] Remote Computing / Boa addon #171
Conversation
felliott
force-pushed
the
feature/boa-addon
branch
3 times, most recently
from
a6bc04c to
ba18c3f
Compare
| operations against the service and to aggregate accounts under a known user. | ||
| """ | ||
|
|
||
| default_root_folder = models.CharField(blank=True) # TODO: don't need this! |
There was a problem hiding this comment.
Maybe remove this if we don;t need it?
I also don't recall any use of default_root_folder in authorized_storage_account and authorized_citation_account
Maybe this can be done during cleanup.
|
|
||
| class ConfiguredRemoteComputingAddon(ConfiguredAddon): | ||
|
|
||
| root_folder = models.CharField(blank=True) # TODO: do we still need this? |
There was a problem hiding this comment.
This is 100% needed for storage and citations to function, but I am not knowledgable enough about remote computing addon
addon_service/credentials/models.py
Outdated
| except ExternalCredentials.authorized_storage_account.RelatedObjectDoesNotExist: | ||
| # FIXME: I have doubts that auth_storage_acct is correct. Shouldn't it be storage? | ||
| # FIXME: Where is citation and remote_computing? | ||
| # FIXME: does that docstring need to be updated? | ||
| return None |
There was a problem hiding this comment.
I suppose that this except clause is redundant
There was a problem hiding this comment.
But on the other hand, authorized account property accounts for all addon types
| @@ -22,6 +22,7 @@ aiohttp = "3.9.3" | |||
| pyjwe = "1.0.0" | |||
| cryptography = "42.0.7" | |||
| boto3 = "1.34" | |||
| boa-api = "^0.1.14" | |||
There was a problem hiding this comment.
Is it mission critical to include boa client and are we sure that we cannot do all of the work using plain http calls?
felliott
force-pushed
the
feature/boa-addon
branch
2 times, most recently
from
d805a81 to
dca84f9
Compare
felliott
changed the title
| serializer = ConfiguredCitationAddonSerializer( | ||
| instance, context={"request": request} | ||
| ) | ||
| else: # computing |
There was a problem hiding this comment.
I know this was the pattern from before, but should we elif hasattr(instance, "configuredcomputingaddon") here and maybe else an error?
| serializer = AuthorizedCitationAccountSerializer( | ||
| instance, context={"request": request} | ||
| ) | ||
| else: # computing |
felliott
force-pushed
the
feature/boa-addon
branch
from
dca84f9 to
6c02876
Compare
| class ConfiguredComputingAddonSerializer(ConfiguredAddonSerializer): | ||
| """api serializer for the `ConfiguredComputingAddon` model""" | ||
|
|
||
| root_folder = serializers.CharField(required=False, allow_blank=True) |
There was a problem hiding this comment.
I beleive this field is redundant for computing addons
There was a problem hiding this comment.
I'm good once Oleh is happy with this. Also, based on this conversation, are we setting the wb_key properly in this PR?
felliott
force-pushed
the
feature/boa-addon
branch
from
6c02876 to
0f0657d
Compare
remote-computing.GLEEP! I forgot to regenerate the migration. The migration is correct, but the order is wrong!ERRRPP! Now there is a broken test.