Skip to content

Conversation

davidmrdavid
Copy link
Collaborator

Bundles V1 is deprecated so I'm adding a step in the initialization file of the module that throws an exception whenever bundles V1 is detected. The result looks as follows:

bundlesvalid

I also changed the setup.py to have an explicit UTF-8 encoding, since now we've added emojis to the README. Without this, it now fails to parse the README, which is necessary for publishing the package

@davidmrdavid
Copy link
Collaborator Author

@anthonychu , Just making sure you're ok with this approach. I found it a bit cleaner than doing it in setup.py and works in more scenarios than just at installation time.

@davidmrdavid
Copy link
Collaborator Author

Sorry for the messy git history here, I accidentally creating this branch with master / main as it's source instead of dev, which is why it has a strange history.

Copy link

@ConnorMcMahon ConnorMcMahon left a comment

Choose a reason for hiding this comment

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

Mostly good, a few nits.

# We do a best-effort attempt to detect bundles V1
# This is the string hard-coded into the bundles V1 template in VSCode
if version_range == "[1.*, 2.0.0)":
message = "Bundles V1 is deprecated. Please update to Bundles V2 in your `host.json`."\

Choose a reason for hiding this comment

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

small tweak: "Durable Python does not support Bundles V1."

The current statement makes it seem like Bundles V1 are deprecated functions wide.

return
# We do a best-effort attempt to detect bundles V1
# This is the string hard-coded into the bundles V1 template in VSCode
if version_range == "[1.*, 2.0.0)":

Choose a reason for hiding this comment

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

I think some small subset of customers may have more specific version ranges, like "[1.2, 2.0.0)" due to pinning during regressions. It's an edge case, but we may want to ask @narensoni if we should cover it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll merge this for now, since it's better than nothing, but track this follow-up here: #247

@davidmrdavid davidmrdavid merged commit d8f2c06 into dev Dec 18, 2020
@davidmrdavid davidmrdavid deleted the dajusto/validate-bundles branch December 18, 2020 22:57
@tamathew
Copy link

Can you also update the template's code's(The one which is automatically getting generated when we select template while creating az function) host.json to use "version": "[2.*, 3.0.0)". By default it is V1 and it took me a while to figure out that.

@davidmrdavid
Copy link
Collaborator Author

Hi @tmathew1000! Ah yes, this is something we're actively working on. Unfortunately, the host.json's contents are, as per my understanding, global to the app creation process so, at the moment, I need buy-in from the larger Functions team before updating the autogenerated host.json file.

I'm looking to improve this state of affairs, ideally by allow a host.json to be overriden / informed by your template selection; it's looking like that will be a longer term project though.

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.

3 participants