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

[11.0][FIX] github_connector keyerror 'blog' #34

Merged
merged 5 commits into from
Feb 2, 2019
Merged

[11.0][FIX] github_connector keyerror 'blog' #34

merged 5 commits into from
Feb 2, 2019

Conversation

enriquemartin
Copy link
Contributor

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

imagen

Current behavior before PR:

Synchronization gets blog key on get_odoo_data_from_github but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
@pedrobaeza pedrobaeza added this to the 11.0 milestone Jan 29, 2019
@pedrobaeza
Copy link
Member

I don't know how you get to that, but what we should do is to restrict the list of vals to valid fields, or directly ask before if the field exists.

@elicoidal
Copy link

Thanks for reporting but I tend to agree with @pedrobaeza otherwise, every time a new field appears in the framework sync we will have to update the code
Can you dig a little bit more on a more long term solution?

@enriquemartin
Copy link
Contributor Author

I don't know how you get to that, but what we should do is to restrict the list of vals to valid fields, or directly ask before if the field exists.

I just synced an organization (OCA) on a new database and the error happened.

Anyway I made some research and I saw that in previous version of this addon, there was a mapping of field-key. Here

In the migration this mapping has been replaced by a list of keys directly, except avatar_url.
blog key contains data of the organization (web url) and it should be saved in an Odoo field; maybe the best option is to recover the mapping of the previous version or at least map the blog key to the corresponding field.

I see now that blog key should be mapped on website_url field as in the previous version of the addon. I make some changes and upload them.

Copy link

@elicoidal elicoidal left a comment

Choose a reason for hiding this comment

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

LGTM with a question
Please bump the version number

github_connector/models/github_organization.py Outdated Show resolved Hide resolved
@pedrobaeza
Copy link
Member

@enriquemartin I think the change was done because sometimes other keys are not present, so the previous iterative process seems OK for me, but you only have to add a mapping dictionary with the GH label and the field name, that it will be the same for all except the blog one. Another option is to add an if with that specific condition.

@enriquemartin
Copy link
Contributor Author

@pedrobaeza what I see proper is to create a method on abstract model abstract.github.model to get the mapping dict. Call it on get_odoo_data_from_github for better inheritance. Something like that?

    @api.model
    def get_conversion_dict(self):
        """Prepare function that map Github fields to Odoo fields"""
        return {
            'github_id_external': 'id',
            'github_url': 'html_url',
            'github_login': self.github_login_field(),
            'github_create_date': 'created_at',
            'github_write_date': 'updated_at',
        }

    @api.model
    def get_odoo_data_from_github(self, data):
        """Prepare function that map Github data to create in Odoo"""
        map_dict = self.get_conversion_dict()
        res = {}
        for k, v in map_dict.items():
            if hasattr(self, v):
                res.update({v: data[k]})
        res.update({'github_last_sync_date': fields.Datetime.now()})
        return res

@pedrobaeza
Copy link
Member

Great, this seems a good idea.

@pedrobaeza
Copy link
Member

Only a pylint error:

github_connector/models/github.py:158: [W8106(method-required-super), Github.create] Missing `super` call in "create" method.

Copy link

@j-zaballa j-zaballa left a comment

Choose a reason for hiding this comment

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

Looks good to me. The Pylint error seems to predate this PR. I don't know what is the way to go in these cases.

@pedrobaeza
Copy link
Member

Well, the error is there because you always must to call super in a inherit.

@j-zaballa
Copy link

Well, the error is there because you always must to call super in a inherit.

Yes. I just meant that the error is not related to this pull request and I don't know if PRs are supposed to address previous errors :)

@pedrobaeza
Copy link
Member

@j-zaballa are you sure? The module is the same, and I would say is due to these changes.

@enriquemartin
Copy link
Contributor Author

@pedrobaeza the error is on line 158 of github_connector/models/github.py file.
This PR does not make any changes on that file; do I fix it anyway?

@pedrobaeza
Copy link
Member

Yes, please 🙏

@enriquemartin
Copy link
Contributor Author

@pedrobaeza Github class inherits from object and object class does not have a create method. That is, the class is defining the method, not overriding it.
I guess that the tests check that there is a super call in all create methods of odoo models, but Github class is not a odoo model.

@pedrobaeza
Copy link
Member

OK, I see, @moylop260 can we consider this as a bug in pylint-odoo?

@pedrobaeza
Copy link
Member

So the error is another, Moi?

@pedrobaeza pedrobaeza merged commit c399f9f into OCA:11.0 Feb 2, 2019
cristinamartinrod pushed a commit to Tecnativa/interface-github that referenced this pull request Feb 13, 2019
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
Tardo pushed a commit to Tecnativa/interface-github that referenced this pull request Jan 29, 2020
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
Tardo pushed a commit to Tecnativa/interface-github that referenced this pull request Mar 3, 2020
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
ernestotejeda pushed a commit to Tecnativa/interface-github that referenced this pull request Mar 16, 2020
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
ernestotejeda pushed a commit to Tecnativa/interface-github that referenced this pull request Mar 16, 2020
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
joao-p-marques pushed a commit to Tecnativa/interface-github that referenced this pull request Jun 22, 2021
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
OpenCode pushed a commit to OpenCode/interface-github that referenced this pull request Jun 22, 2022
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
victoralmau pushed a commit to Tecnativa/interface-github that referenced this pull request Oct 11, 2022
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
victoralmau pushed a commit to Tecnativa/interface-github that referenced this pull request Nov 15, 2022
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
victoralmau pushed a commit to Tecnativa/interface-github that referenced this pull request Nov 15, 2022
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
carolinafernandez-tecnativa pushed a commit to Tecnativa/interface-github that referenced this pull request Jan 8, 2024
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
victoralmau pushed a commit to Tecnativa/interface-github that referenced this pull request Nov 8, 2024
[FIX] github_connector: keyerror 'blog'

Description of the issue/feature this PR addresses:

On new database error when syncing new github organization (example: OCA).
Error traceback: KeyError: 'blog'

Current behavior before PR:

Synchronization gets blog key on `get_odoo_data_from_github` but blog field does not exist.
When it tries to match the key with model attribute we get key error.

Desired behavior after PR is merged:

Get blog key from github data and show it on form view.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants