Skip to content

[12.0][MIG] keychain - WIP - #39

Closed
hparfr wants to merge 17 commits into
OCA:12.0from
akretion:12.0-mig-keychain
Closed

hparfr wants to merge 17 commits into
OCA:12.0from
akretion:12.0-mig-keychain

Conversation

@hparfr

@hparfr hparfr commented Oct 1, 2018

Copy link
Copy Markdown
Contributor

No description provided.

@hparfr hparfr changed the title 12.0 mig keychain [12.0][MIG] keychain Oct 1, 2018
@hparfr hparfr changed the title [12.0][MIG] keychain [12.0][MIG] keychain - WIP Oct 1, 2018
@pedrobaeza pedrobaeza added this to the 12.0 milestone Oct 1, 2018
@pedrobaeza

Copy link
Copy Markdown
Member

Hi, Raph, can you Travis and runbot status?

@hparfr
hparfr force-pushed the 12.0-mig-keychain branch from 96fcf27 to 3d4ef97 Compare October 1, 2018 13:43
@hparfr

hparfr commented Oct 1, 2018

Copy link
Copy Markdown
Contributor Author

OCB branch is good now, thanks Pedro

@OCA-git-bot OCA-git-bot mentioned this pull request Oct 1, 2018
19 tasks
@pedrobaeza

Copy link
Copy Markdown
Member

Can you please attend these warnings (it's a new Odoo check):

2018-10-01 13:48:02,744 150 WARNING openerp_test odoo.models: The model keychain.account has no _description
2018-10-01 13:48:02,745 150 WARNING openerp_test odoo.models: The model keychain.backend has no _description

hparfr added 2 commits October 2, 2018 09:47
[11.0] keychain: improve ACL and allow storing ssh keys as password OCA#30
@rvalyi

rvalyi commented May 23, 2019

Copy link
Copy Markdown
Member

so if I'am right, this PR is superseded by this one #39, right?
If so, may be we should close this PR and mention the keychain origin in the new data_encryption module. cc @hparfr @florian-dacosta

@rousseldenis rousseldenis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Some comments

@hparfr Could we try to move forward on this one ?

try:
return self._decode_password(self.password)
except Warning as warn:
raise Warning(_(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this is incorrect to raise not ORM exceptions.

data = fields.Text(help="Additionnal data as json")

def _compute_password(self):
# Only needed in v8 for _description_searchable issues

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be removed

"""At this time there is no namespace set."""
if not vals.get('data') and not self.data:
vals['data'] = self._serialize_data(self._init_data())
return super(KeychainAccount, self).write(vals)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be simplified

Suggested change
return super(KeychainAccount, self).write(vals)
return super().write(vals)

@florian-dacosta

Copy link
Copy Markdown
Contributor

@rousseldenis
Maybe we should close this PR.
Indeed, we have refactored the module and would use : OCA/server-env#24
which is based on server_environment
So, I don't think we will continue this PR.

@rousseldenis

Copy link
Copy Markdown
Contributor

@florian-dacosta Ok, thanks.

@hparfr Could you close ?

@hparfr hparfr closed this Jun 6, 2019
@hparfr
hparfr deleted the 12.0-mig-keychain branch June 6, 2019 09:47
SiesslPhillip pushed a commit to grueneerde/OCA-server-auth that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-auth (9.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.