Skip to content

Comments

Update WasbHook to reflect preference for unprefixed extra#27024

Merged
dstandish merged 4 commits intoapache:mainfrom
astronomer:azure-wasb-remove-extra-prefix
Oct 22, 2022
Merged

Update WasbHook to reflect preference for unprefixed extra#27024
dstandish merged 4 commits intoapache:mainfrom
astronomer:azure-wasb-remove-extra-prefix

Conversation

@dstandish
Copy link
Contributor

Since 2.3 we don't need the extra prefix to edit custom fields with UI form. From 2.5 we won't need to use the prefix in the ui form behaviors method either.

Since 2.3 we don't need the extra prefix to edit custom fields with UI form.  From 2.5 we won't need to use the prefix in the ui form behaviors method either.
f"when using this method."
)
if field_name in extra_dict:
return extra_dict[field_name] or None
Copy link
Contributor

@rajaths010494 rajaths010494 Oct 14, 2022

Choose a reason for hiding this comment

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

Is None needed to be returned when the key is present in the dict? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could be empty string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i like to treat empty string as none in this context because UI creates all custom fields with empty string even if not supplied

Copy link
Contributor

Choose a reason for hiding this comment

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

ok


def _get_field(self, extra_dict, field_name):
prefix = 'extra__wasb__'
if field_name.startswith('extra_'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as the previous PR review comment, can we check for the exact key extra__

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

)
if field_name in extra_dict:
return extra_dict[field_name] or None
return extra_dict.get(f"{prefix}{field_name}") or None
Copy link
Contributor

Choose a reason for hiding this comment

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

we also write like this extra_dict.get(f"{prefix}{field_name}", None) , its upto you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

difference is empty string

@dstandish dstandish merged commit d51de50 into apache:main Oct 22, 2022
@dstandish dstandish deleted the azure-wasb-remove-extra-prefix branch October 22, 2022 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants