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

[🚀 Feature]: Grid properly matches enable download support #11658

Closed
titusfortner opened this issue Feb 14, 2023 · 2 comments
Closed

[🚀 Feature]: Grid properly matches enable download support #11658

titusfortner opened this issue Feb 14, 2023 · 2 comments

Comments

@titusfortner
Copy link
Member

Feature and motivation

This is an extension of #11656 but I wanted to separate it out.

We know we want the Grid to set whether it allows downloads. Right now we're doing a flag where we have users specify the directory. If we are creating that directory for users, that won't be necessary. We can either keep the flag and change it to --enable-downloads or we can do whatever we do now by requiring a match of se:enableDownloads capability. I kind of like the latter approach because if a user requests a session with downloads enabled, but the Hub doesn't find a node with it turned on, it would be nice to get a clear error message that this is the case.

Usage example

backend implementation

@krmahadevan krmahadevan self-assigned this Feb 15, 2023
krmahadevan added a commit to krmahadevan/selenium that referenced this issue Feb 23, 2023
Fixes SeleniumHQ#11656 SeleniumHQ#11658

Following has been done:

* Specify the default base directory into which 
all downloads at a node will go into via the flag 
“—-base-dir-downloads”. If this flag does not have 
a value then we default to user’s home.
* Turn ON managing download folders via the flag 
“-—enable-manage-downloads”
* Enabled support for Chrome|Edge|Firefox browsers.
* File downloads will be done only in a session aware 
directory for a given web driver session. After session
is killed, the directory gets cleaned up as well.
krmahadevan added a commit to krmahadevan/selenium that referenced this issue Feb 24, 2023
Fixes SeleniumHQ#11656 SeleniumHQ#11658

Following has been done:

* Specify the default base directory into which 
all downloads at a node will go into via the flag 
“—-base-dir-downloads”. If this flag does not have 
a value then we default to user’s home.
* Turn ON managing download folders via the flag 
“-—enable-manage-downloads”
* Enabled support for Chrome|Edge|Firefox browsers.
* File downloads will be done only in a session aware 
directory for a given web driver session. After session
is killed, the directory gets cleaned up as well.
krmahadevan added a commit to krmahadevan/selenium that referenced this issue Mar 1, 2023
Fixes SeleniumHQ#11656 SeleniumHQ#11658

Following has been done:

* Specify the default base directory into which 
all downloads at a node will go into via the flag 
“—-base-dir-downloads”. If this flag does not have 
a value then we default to user’s home.
* Turn ON managing download folders via the flag 
“-—enable-manage-downloads”
* Enabled support for Chrome|Edge|Firefox browsers.
* File downloads will be done only in a session aware 
directory for a given web driver session. After session
is killed, the directory gets cleaned up as well.
krmahadevan added a commit to krmahadevan/selenium that referenced this issue Mar 7, 2023
Fixes SeleniumHQ#11656 SeleniumHQ#11658

Following has been done:

* Specify the default base directory into which 
all downloads at a node will go into via the flag 
“—-base-dir-downloads”. If this flag does not have 
a value then we default to user’s home.
* Turn ON managing download folders via the flag 
“-—enable-manage-downloads”
* Enabled support for Chrome|Edge|Firefox browsers.
* File downloads will be done only in a session aware 
directory for a given web driver session. After session
is killed, the directory gets cleaned up as well.
krmahadevan added a commit to krmahadevan/selenium that referenced this issue Mar 10, 2023
Fixes SeleniumHQ#11656 SeleniumHQ#11658

Following has been done:

* Specify the default base directory into which 
all downloads at a node will go into via the flag 
“—-base-dir-downloads”. If this flag does not have 
a value then we default to user’s home.
* Turn ON managing download folders via the flag 
“-—enable-manage-downloads”
* Enabled support for Chrome|Edge|Firefox browsers.
* File downloads will be done only in a session aware 
directory for a given web driver session. After session
is killed, the directory gets cleaned up as well.
diemol added a commit that referenced this issue Mar 15, 2023
* [Grid] Support auto downloads in Grid

Fixes #11656 #11658

Following has been done:

* Turn ON managing download folders via the flag 
“-—enable-manage-downloads”
* Enabled support for Chrome|Edge|Firefox browsers.
* File downloads will be done only in a session aware 
directory for a given web driver session. After session
is killed, the directory gets cleaned up as well.

* [grid] Renaming to manage downloads enabled

and removing out of scope logic to determine
Node match and client side validations

* [grid] Using the temp file system utility

With this, it will be transparent for the user
where files are written, and since we use the
caches then the deletion happens when the session
is closed.

Also, we do not need the `--base-dir-downloads`
parameter.

* [grid] Adding a cleanup executor for downloaded files

* [grid] Adding e2e test and fixing bug found while adding test

* [grid] Removing test

---------

Co-authored-by: Diego Molina <[email protected]>
Co-authored-by: Diego Molina <[email protected]>
@diemol
Copy link
Member

diemol commented Mar 15, 2023

Closed via #11702

@diemol diemol closed this as completed Mar 15, 2023
alpatron pushed a commit to alpatron/selenium that referenced this issue Mar 15, 2023
* [Grid] Support auto downloads in Grid

Fixes SeleniumHQ#11656 SeleniumHQ#11658

Following has been done:

* Turn ON managing download folders via the flag 
“-—enable-manage-downloads”
* Enabled support for Chrome|Edge|Firefox browsers.
* File downloads will be done only in a session aware 
directory for a given web driver session. After session
is killed, the directory gets cleaned up as well.

* [grid] Renaming to manage downloads enabled

and removing out of scope logic to determine
Node match and client side validations

* [grid] Using the temp file system utility

With this, it will be transparent for the user
where files are written, and since we use the
caches then the deletion happens when the session
is closed.

Also, we do not need the `--base-dir-downloads`
parameter.

* [grid] Adding a cleanup executor for downloaded files

* [grid] Adding e2e test and fixing bug found while adding test

* [grid] Removing test

---------

Co-authored-by: Diego Molina <[email protected]>
Co-authored-by: Diego Molina <[email protected]>
Copy link

github-actions bot commented Dec 9, 2023

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants