feat: prevent writer framework version mismatch between local & deploy - WF-67 #626

FabienArcellier · 2024-11-11T10:34:34Z

when using writer deploy, it check the version set in poetry during a cloud deployment is the one used in development

when using writer run, it show a warning when the version used is obsolete regards to the development version

when using writer edit, it show a warning when the version used is obsolete regards to the development version

src/writer/wf_project.py

ramedina86 · 2024-11-11T10:56:48Z

Hey, I didn't read the whole PR, but relying only on the version pinned in requirements.txt or pyproject.toml is problematic. And I think that's the case.

As we discussed with @raaymax requirements.txt will be optional. Also, there should be no need to pin a Framework version. We should assume that if nothing is specified the latest stable version in PyPi is used.

What I was planning, also why I added

include the version in the front end for easier troubleshooting (probably not visible, but easily checked via chrome Dev tools/Inspect Element)

is... serve the version via an endpoint, e.g. myapp.com/metadata

then compare that during deployment, AFTER the app has been deployed and issue a warning if it's not the same. I guess the main disadvantage of this it's that it's Writer-specific, but I think it covers this need very well and gives you a much needed way of checking the actual Framework version from the frontend.

ramedina86 · 2024-11-11T11:04:36Z

For version comparison the best option seems to be https://packaging.pypa.io/en/latest/version.html#packaging.version.Version

FabienArcellier · 2024-11-11T11:33:55Z

For version comparison the best option seems to be https://packaging.pypa.io/en/latest/version.html#packaging.version.Version

I will move to it

FabienArcellier · 2024-11-11T11:38:26Z

then compare that during deployment, AFTER the app has been deployed and issue a warning if it's not the same. I guess the main disadvantage of this it's that it's Writer-specific, but I think it covers this need very well and gives you a much needed way of checking the actual Framework version from the frontend.

Currently it is doing that. It show a warning in the application logs. It not depends on anything for that. I will look for a way to show the warning in writer deploy when it happens.

ramedina86 · 2024-11-11T12:01:51Z

Please make sure to get the effective version and not search for it on requirements.txt or pyproject.toml, otherwise I'm not too sure how we can cover the scenario where writer version isn't specified.

FabienArcellier · 2024-11-11T12:08:48Z

Please make sure to get the effective version and not search for it on requirements.txt or pyproject.toml, otherwise I'm not too sure how we can cover the scenario where writer version isn't specified.

It search for poetry only for one specific edge case (poetry on cloud deploy) to give the user the best recommandation. In the other cases, It rely on the comparison package version (VERSION) and ./wf/metadata.json. It just give hint to user where to declare it's dependencies depending of the present strategy

ramedina86 · 2024-11-11T12:10:20Z

Ah sorry, that's great!

FabienArcellier · 2024-11-11T12:12:55Z

tests/backend/fixtures/obsoletes/poetry.lock

@@ -0,0 +1,1209 @@
+# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand.


FabienArcellier · 2024-11-11T12:13:20Z

apps/default/poetry.lock

@@ -0,0 +1,1209 @@
+# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand.


FabienArcellier · 2024-11-11T19:54:35Z

src/writer/ss_types.py

@@ -1,3 +1,4 @@
+import dataclasses


rollback those changes

FabienArcellier · 2024-11-14T09:04:39Z

src/writer/deploy.py

@@ -47,6 +52,16 @@ def deploy(path, api_key, env, verbose, force):
    if not os.path.isdir(abs_path):
        raise click.ClickException("A path to a folder containing a Writer Framework app is required. For example: writer cloud deploy my_app")

+    ignore_poetry_version_check = int(os.getenv("IGNORE_POETRY_VERSION_CHECK", '0')) == 1


add WRITER_WF_ , check if the variable is present instead of checking value

ramedina86 · 2024-11-22T09:23:04Z

src/writer/deploy.py

@@ -47,6 +52,16 @@ def deploy(path, api_key, env, verbose, force):
    if not os.path.isdir(abs_path):
        raise click.ClickException("A path to a folder containing a Writer Framework app is required. For example: writer cloud deploy my_app")


Very small one but now that I spotted it it bothers me 😅 Can you use either click.ClickException or ClickException?

ramedina86 · 2024-11-22T09:23:41Z

src/writer/deploy.py

@@ -47,6 +52,16 @@ def deploy(path, api_key, env, verbose, force):
    if not os.path.isdir(abs_path):
        raise click.ClickException("A path to a folder containing a Writer Framework app is required. For example: writer cloud deploy my_app")

+    ignore_poetry_version_check = int(os.getenv("IGNORE_POETRY_VERSION_CHECK", '0')) == 1


Can we go ahead with this change that we discussed?

ramedina86 · 2024-12-10T16:07:39Z

Can this be merged without any changes in the Writer's deployment service repo?

* fix: comment from review * feat: warn user about consistency version when running the app * refact: improve wf_project module readability * feat: implement poetry verification

FabienArcellier requested review from raaymax and ramedina86 November 11, 2024 10:34

FabienArcellier changed the title ~~prevent writer framework version mismatch local cloud - Wf 67~~ Prevent writer framework version mismatch between local & cloud - Wf 67 Nov 11, 2024

FabienArcellier force-pushed the WF-67-prevent-writer-framework-version-mismatch-local-cloud branch from 4635266 to 4cba79c Compare November 11, 2024 10:41

FabienArcellier commented Nov 11, 2024

View reviewed changes

src/writer/wf_project.py Outdated Show resolved Hide resolved

FabienArcellier commented Nov 11, 2024

View reviewed changes

src/writer/wf_project.py Outdated Show resolved Hide resolved

FabienArcellier force-pushed the WF-67-prevent-writer-framework-version-mismatch-local-cloud branch from 4cba79c to 3bd8645 Compare November 11, 2024 10:44

FabienArcellier self-assigned this Nov 11, 2024

FabienArcellier changed the title ~~Prevent writer framework version mismatch between local & cloud - Wf 67~~ Prevent writer framework version mismatch between local & cloud - WF-67 Nov 11, 2024

FabienArcellier changed the title ~~Prevent writer framework version mismatch between local & cloud - WF-67~~ Prevent writer framework version mismatch between local & deploy - WF-67 Nov 11, 2024

FabienArcellier commented Nov 11, 2024

View reviewed changes

src/writer/ss_types.py Outdated

@@ -1,3 +1,4 @@

import dataclasses

Copy link

Collaborator Author

FabienArcellier Nov 11, 2024

Choose a reason for hiding this comment

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

rollback those changes

FabienArcellier force-pushed the WF-67-prevent-writer-framework-version-mismatch-local-cloud branch 2 times, most recently from c3bf7e4 to aa2de38 Compare November 11, 2024 20:06

FabienArcellier changed the title ~~Prevent writer framework version mismatch between local & deploy - WF-67~~ feat: prevent writer framework version mismatch between local & deploy - WF-67 Nov 11, 2024

FabienArcellier added the enhancement New feature or request label Nov 11, 2024

FabienArcellier force-pushed the WF-67-prevent-writer-framework-version-mismatch-local-cloud branch from aa2de38 to 987b546 Compare November 13, 2024 17:25

FabienArcellier commented Nov 14, 2024

View reviewed changes

FabienArcellier force-pushed the WF-67-prevent-writer-framework-version-mismatch-local-cloud branch from 987b546 to e5d4184 Compare November 21, 2024 08:15

ramedina86 reviewed Nov 22, 2024

View reviewed changes

FabienArcellier force-pushed the WF-67-prevent-writer-framework-version-mismatch-local-cloud branch from e5d4184 to 44ed26a Compare November 24, 2024 15:00

FabienArcellier force-pushed the WF-67-prevent-writer-framework-version-mismatch-local-cloud branch from 44ed26a to f75f6a7 Compare December 8, 2024 14:24

FabienArcellier marked this pull request as ready for review December 9, 2024 13:40

raaymax approved these changes Dec 10, 2024

View reviewed changes

ramedina86 marked this pull request as draft December 10, 2024 16:49

FabienArcellier force-pushed the WF-67-prevent-writer-framework-version-mismatch-local-cloud branch 2 times, most recently from 81545f4 to 4285be8 Compare December 16, 2024 14:50

FabienArcellier added 2 commits December 19, 2024 09:37

feat: prevent Writer Framework version mismatch local/cloud

ec8c1af

* fix: comment from review * feat: warn user about consistency version when running the app * refact: improve wf_project module readability * feat: implement poetry verification

feat: prevent Writer Framework version mismatch local/cloud

83f22bf

FabienArcellier force-pushed the WF-67-prevent-writer-framework-version-mismatch-local-cloud branch from 4285be8 to 83f22bf Compare December 19, 2024 15:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: prevent writer framework version mismatch between local & deploy - WF-67 #626

feat: prevent writer framework version mismatch between local & deploy - WF-67 #626

FabienArcellier commented Nov 11, 2024 •

edited

Loading

ramedina86 commented Nov 11, 2024

ramedina86 commented Nov 11, 2024

FabienArcellier commented Nov 11, 2024

FabienArcellier commented Nov 11, 2024 •

edited

Loading

ramedina86 commented Nov 11, 2024

FabienArcellier commented Nov 11, 2024 •

edited

Loading

ramedina86 commented Nov 11, 2024

FabienArcellier Nov 11, 2024

FabienArcellier Nov 11, 2024

FabienArcellier Nov 11, 2024

FabienArcellier Nov 14, 2024

ramedina86 Nov 22, 2024

ramedina86 Nov 22, 2024

ramedina86 commented Dec 10, 2024

		@@ -0,0 +1,1209 @@
		# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand.

		@@ -47,6 +52,16 @@ def deploy(path, api_key, env, verbose, force):
		if not os.path.isdir(abs_path):
		raise click.ClickException("A path to a folder containing a Writer Framework app is required. For example: writer cloud deploy my_app")

feat: prevent writer framework version mismatch between local & deploy - WF-67 #626

Are you sure you want to change the base?

feat: prevent writer framework version mismatch between local & deploy - WF-67 #626

Conversation

FabienArcellier commented Nov 11, 2024 • edited Loading

ramedina86 commented Nov 11, 2024

ramedina86 commented Nov 11, 2024

FabienArcellier commented Nov 11, 2024

FabienArcellier commented Nov 11, 2024 • edited Loading

ramedina86 commented Nov 11, 2024

FabienArcellier commented Nov 11, 2024 • edited Loading

ramedina86 commented Nov 11, 2024

FabienArcellier Nov 11, 2024

Choose a reason for hiding this comment

FabienArcellier Nov 11, 2024

Choose a reason for hiding this comment

FabienArcellier Nov 11, 2024

Choose a reason for hiding this comment

FabienArcellier Nov 14, 2024

Choose a reason for hiding this comment

ramedina86 Nov 22, 2024

Choose a reason for hiding this comment

ramedina86 Nov 22, 2024

Choose a reason for hiding this comment

ramedina86 commented Dec 10, 2024

FabienArcellier commented Nov 11, 2024 •

edited

Loading

FabienArcellier commented Nov 11, 2024 •

edited

Loading

FabienArcellier commented Nov 11, 2024 •

edited

Loading